[SPARK-58208][SQL] Deep-copy stateful expressions before optimization - #57980
[SPARK-58208][SQL] Deep-copy stateful expressions before optimization#57980marcuslin123 wants to merge 1 commit into
Conversation
| @@ -308,15 +308,21 @@ class QueryExecution( | |||
|
|
|||
| def assertCommandExecuted(): Unit = commandExecuted | |||
|
|
|||
| private def cloneWithFreshStatefulExpressions(plan: LogicalPlan): LogicalPlan = { | |||
| plan.clone().transformWithSubqueries { | |||
| case node => node.mapExpressions(_.freshCopyIfContainsStatefulExpression()) | |||
There was a problem hiding this comment.
mapExpressions uses fastEquals to decide whether an expression actually changed:
val newE = f(e)
if (newE.fastEquals(e)) e // structural equality — not reference equality
else { changed = true; newE }
This works for NamedLambdaVariable because value: AtomicReference is a constructor argument — two NLVs with different AtomicReference instances are not structurally equal, so fastEquals returns false and the fresh copy sticks.
But for expressions like RegExpReplace, StringTranslate, FormatNumber (SPARK-58204), the mutable state lives in a @transient var — not in the constructor. A fresh copy of RegExpReplace has identical constructor args (subject, regexp, replacement), so fastEquals returns true and the fresh copy is silently discarded — the deep-copy becomes a no-op for those expressions.
In practice this is not a correctness issue today — ConvertToLocalRelation has its own direct freshCopyIfContainsStatefulExpression() call that covers driver-side evaluation, and non-local plans serialize fresh copies to each executor task. But it is a latent gap for future stateful expressions.
A simple fix is to use reference equality (ne) instead:
// change detection with ne instead of fastEquals
val newE = e.freshCopyIfContainsStatefulExpression()
if (newE ne e) { changed = true; newE } else e
What changes were proposed in this pull request?
This PR deep-copies stateful expressions before they are evaluated by the optimizer.
Specifically, it:
QueryExecution.optimizedPlaninvokes the optimizer;ConvertToLocalRelationevaluates project and filter expressions; andNamedLambdaVariable.Why are the changes needed?
Logical plans can be shared by DataFrames derived from the same base DataFrame. Although
QueryExecutioncloned the plan before optimization, stateful expressions such asNamedLambdaVariablecould retain shared mutable state across those clones. Concurrent optimization could therefore mutate the same expression state from multiple threads and silently corrupt query results.ConvertToLocalRelationhad the same issue because it evaluated expressions from the shared plan directly.Does this PR introduce any user-facing change?
Yes. Concurrent actions on DataFrames derived from a shared logical plan no longer risk silent result corruption caused by shared mutable expression state.
How was this patch tested?
Added regression tests to
ConvertToLocalRelationSuiteandQueryExecutionSuite.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Codex (GPT-5). Codex was used for code assistance and review; the patch was not entirely generated by Codex.