cloud-fan commented on code in PR #58045:
URL: https://github.com/apache/spark/pull/58045#discussion_r3797825399
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/RewriteWithExpression.scala:
##########
@@ -183,6 +215,15 @@ object RewriteWithExpression extends Rule[LogicalPlan] {
val newExpr = c.withNewAlwaysEvaluatedInputs(newAlwaysEvaluatedInputs)
// Use transformUp to handle nested With.
newExpr.transformUpWithPruning(_.containsPattern(WITH_EXPRESSION)) {
+ case w @ With(_, defs) if defs.exists(shouldPreEvaluateInBranch(_,
commonExprIdSet)) =>
Review Comment:
**Non-blocking:**
Could we preserve branch-local lazy evaluation instead of moving the
definition into an eager `Project`? Hoisting `rand(42)` consumes draws on
untaken rows, so later taken rows receive different reproducible values; the
`defs.exists` guard can also hoist an unsafe sibling such as `RandStr(-1, 0)`.
Please investigate lazy per-row memoization inside the conditional branch—once
when taken and zero times otherwise—and cover RNG advancement plus mixed
definitions in tests.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/RewriteWithExpressionSuite.scala:
##########
@@ -250,6 +251,79 @@ class RewriteWithExpressionSuite extends PlanTest {
)
}
+ test("SPARK-58818: nondeterministic common expression in a conditional
branch") {
+ val a = testRelation.output.head
+ // The shape `input BETWEEN lower AND upper` builds: the reference appears
twice.
Review Comment:
**Nit:**
```suggestion
// The shape built for `input BETWEEN lower AND upper` references the
input twice.
```
--
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]
