vinodkc commented on code in PR #57980:
URL: https://github.com/apache/spark/pull/57980#discussion_r3772376574
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala:
##########
@@ -308,15 +308,21 @@ class QueryExecution(
def assertCommandExecuted(): Unit = commandExecuted
+ private def cloneWithFreshStatefulExpressions(plan: LogicalPlan):
LogicalPlan = {
+ plan.clone().transformWithSubqueries {
+ case node =>
node.mapExpressions(_.freshCopyIfContainsStatefulExpression())
Review 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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
