pratham76 commented on code in PR #58656:
URL: https://github.com/apache/spark/pull/58656#discussion_r4040695643
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala:
##########
@@ -454,6 +470,70 @@ object RewritePredicateSubquery extends Rule[LogicalPlan]
with PredicateHelper {
}
(newExprs.reduceOption(And), newPlan, introducedAttrs.toSeq)
}
+
+ /**
+ * Returns true if `e` references any of the attributes produced by `plan`.
+ */
+ private def referencesPlan(e: Expression, plan: LogicalPlan): Boolean = {
+ e.references.intersect(plan.outputSet).nonEmpty
+ }
+
+ /**
+ * Rewrites the existential sub-queries that are nested in the correlated
predicates which
+ * [[PullupCorrelatedPredicates]] hoisted out of a predicate sub-query, and
which therefore end
+ * up in the condition of the semi/anti join that replaces that sub-query.
+ *
+ * A hoisted predicate can carry a nested existential sub-query out of the
sub-query plan,
+ * because a correlated predicate is hoisted as a whole when it is a
disjunction. For example
+ *
+ * SELECT * FROM t1 WHERE EXISTS (
+ * SELECT 1 FROM t2 WHERE t1.a = t2.c1 OR t2.c1 IN (SELECT col1 FROM t3))
+ *
+ * hoists `a = c1 OR c1 IN (SELECT col1 FROM t3)` into the join condition.
Such a nested
+ * sub-query must be rewritten against the plan that produces the attributes
it references:
+ * the one above references `c1`, which is produced by the sub-query plan
and not by the outer
+ * plan, so its existence join has to be built on top of the sub-query plan.
Building it on top
+ * of the outer plan instead yields a join whose condition references an
attribute that neither
+ * of its children can produce (SPARK-59351).
+ *
+ * A nested sub-query that references both plans cannot be rewritten into an
existence join on
+ * either side, so it is left in the join condition, where both plans are in
scope. Such a
+ * sub-query is necessarily uncorrelated, as being correlated to the outer
plan as well would
+ * require two levels of correlation, which the Analyzer rejects. It is then
either planned as
+ * an in-subquery filter or reported as unsupported by the rewrite of
predicate sub-queries in
+ * join conditions.
Review Comment:
You are right, and this is worse than a doc error. I reproduced it exactly
as you described.
With
`spark.sql.optimizer.decorrelatePredicateSubqueriesInJoinPredicate.enabled=false`,
on
`t1(a)=(1,2,3)`, `t2(c1)=(1,8,9)`, `t3(col1)=(3,9)`:
```sql
SELECT * FROM t1 WHERE EXISTS (
SELECT 1 FROM t2 WHERE a = c1 OR a IN (SELECT col1 FROM t3 WHERE col1 =
c1));
```
| | result |
|---|---|
| PostgreSQL / DuckDB | `1` |
| master | `INTERNAL_ERROR` (attribute not found) |
| this PR, before the fixup | **`1, 3`** — silently wrong |
The plan shows why: the leftover keeps its correlation in the list query,
```
+- Join LeftSemi, ((col1#29 = c1#35) OR col1#29 IN (list#32 [c1#35 &&
(col1#36 = c1#35)]))
```
and `PlanSubqueries` then plans it as an uncorrelated `InSubqueryExec`,
dropping `col1 = c1`. As you
noted, `InSubquery` has no `references` override, so `values :+ query` puts
the outer reference in
`references` while the list query stays correlated to the inner query —
exactly the shape my
"necessarily uncorrelated" sentence claimed could not exist. Turning an
error into a wrong answer is
not an acceptable trade, so I took your suggestion and settled it in this PR
rather than leaving it
to the doc:
* a leftover that is **correlated** now raises `0A000`
(`UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.UNSUPPORTED_CORRELATED_EXPRESSION_IN_JOIN_CONDITION`),
because its join condition cannot survive being planned in the join
condition;
* a leftover that is **uncorrelated** has no join condition to lose and is
still left in place.
The check lives in `rewriteExistentialExprInSubqueryPlan`, so it covers the
`NOT IN` branch too, not
only the branches that go through `rewriteExistentialExprInJoinCondition`.
The rejection is
deliberately independent of `decorrelatePredicateSubqueriesInJoinPredicate`:
previously the shape
happened to be caught by the join-condition rewrite under the default confs
and produced a wrong
answer with that conf off, which is why the bug was invisible by default.
The new test asserts
`0A000` under **both** settings of that conf.
Correlation can only be to the sub-query plan here, since correlating to the
outer plan as well
would need two levels of correlation and the Analyzer rejects that — the doc
now says only that,
and no longer claims the leftover is uncorrelated.
--
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]
