pratham76 commented on code in PR #58656:
URL: https://github.com/apache/spark/pull/58656#discussion_r4356960955
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala:
##########
@@ -398,62 +414,568 @@ object RewritePredicateSubquery extends
Rule[LogicalPlan] with PredicateHelper {
(newExpr, newPlan)
}
+ /**
+ * Rewrites the existential sub-queries carried by the condition of an
existence join that this
+ * rule has just built, against whichever of that join's own inputs can
evaluate them.
+ *
+ * [[rewriteExistentialExprWithAttrs]] grafts the condition that
[[PullupCorrelatedPredicates]]
+ * hoisted into a nested sub-query onto the existence join it builds for
that sub-query, and
+ * replaces the sub-query atomically without descending into what the
condition carries. A
+ * correlated existential sub-query carried along references the plan of the
sub-query it was
+ * hoisted out of, which is an input of the join just built and of no plan
seen before it, so
+ * nothing else reaches it: left there, `PlanSubqueries` plans it as an
in-subquery filter and
+ * drops its own hoisted condition, which returns extra rows.
+ *
+ * This is what the handling of predicate sub-queries in join conditions
does for a join the user
+ * wrote, and doing it here too makes the result independent of
+ *
`spark.sql.optimizer.decorrelatePredicateSubqueriesInJoinPredicate.enabled`,
which gates that
+ * handling. The routing recurses, since the joins built here go through it
in turn.
+ *
+ * An uncorrelated sub-query is deliberately left where it is. It has no
hoisted condition to
+ * lose, and `InSubqueryExec` evaluates it with the three-valued semantics
that the `exists`
+ * attribute of an existence join cannot carry.
+ */
+ private def rewriteCarriedExistentialExprs(join: Join): LogicalPlan = {
+ if (!join.condition.exists(_.containsAnyPattern(EXISTS_SUBQUERY,
IN_SUBQUERY))) {
+ return join
+ }
+ val condition = join.condition.get
+ // Only a correlated sub-query has to be consumed here, see above.
+ val carried = condition.collect {
+ case sq @ (_: Exists | _: InSubquery) if hasCorrelatedCondition(sq) => sq
Review Comment:
Confirmed, and this one undoes the claim I made last round: the two
configurations did **not** agree on this shape, only on the correlated one I
had tested.
| | result |
|---|---|
| before, `conf=true` (default) | `1, 2, 3` |
| before, `conf=false` | `1` |
| **now, both** | **rejected, `NESTED_IN_SUBQUERY_WITH_UNKNOWN_RESULT`** |
| DuckDB | `1` |
My premise that `InSubqueryExec` would evaluate an uncorrelated carried IN
three-valued was wrong on this route, for exactly the reason you give: the
top-down pass reaches the existence join in the `case j: Join` arm, which has
no unknown guard, and two-values it under the default configuration.
The router now classifies carried sub-queries with the same rules as
`rewriteExistentialExprInSubqueryPlan`, against this join's inputs instead of
the outer and the sub-query plan: uncorrelated ones are considered too, one
referencing exactly one input is routed, one referencing both is rejected when
correlated or carrying a correlated sub-query, and one referencing both or
neither with no correlation is left in place. The scaladoc no longer says an
uncorrelated carried sub-query is left for `InSubqueryExec`; it now says why
consuming it here is what keeps it under the unknown guard.
On the trade-off you flagged: rejecting an uncorrelated unknown-sensitive
carried IN under both configurations does turn a query that `conf=false`
answers correctly today into an error. I have taken that, since it matches the
first-level policy and leaves no shape where the two configurations disagree.
The truth-only form of the same query is still rewritten and still returns `1`
under both settings, which the new test asserts.
--
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]
