pratham76 commented on code in PR #58656:
URL: https://github.com/apache/spark/pull/58656#discussion_r4072209586
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala:
##########
@@ -414,7 +433,8 @@ object RewritePredicateSubquery extends Rule[LogicalPlan]
with PredicateHelper {
existenceJoin, newCondition, subHint)
introducedAttrs += exists
exists
- case Not(InSubquery(values, ListQuery(sub, _, _, _, conditions,
subHint))) =>
+ case sq @ Not(InSubquery(values, ListQuery(sub, _, _, _, conditions,
subHint)))
+ if canRewrite(sq) =>
val exists = AttributeReference("exists", BooleanType, nullable =
false)()
// Deduplicate conflicting attributes if any.
val newSub = dedupSubqueryOnSelfJoin(newPlan, sub, Some(values))
Review Comment:
Taking the three parts separately.
**(b) is confirmed**, and it is what the new `RewriteSubquerySuite` test
ends up asserting. With the
outer and the subquery plan sharing an attribute, the EXISTS arm reaches
`dedupSubqueryOnSelfJoin`
through `buildJoin` with the condition, and the query fails with
`_LEGACY_ERROR_TEMP_1212`:
```
Found conflicting attributes a#44 in the condition joining outer plan:
LocalRelation [a#44, b#45]
and subplan:
'Join ExistenceJoin(exists#49), ((a#44 = 'col1) AND ('col1 = a#44))
```
As you say, a correlated EXISTS's hoisted condition always names subplan
attributes, so a collision
the condition mentions always lands there. That also bounds what the routing
fix can achieve: it
decides which error appears, not whether the query runs.
**(a) I could not reproduce.** Your `c1 IN (SELECT a FROM t1)` example comes
out of the analyzer
already deduplicated, and with plan change validation on, that query and the
other self-join shapes
I tried — EXISTS, NOT EXISTS and IN over the same relation, with a nested IN
and with a nested
EXISTS — all give plans with no missing input, `duplicateResolved` joins and
`resolved = true`, and
the answers PostgreSQL and DuckDB give. So I have no
`checkIfSameExprIdNotReused` failure to fix,
and I am not treating the concern as unfounded: it is a structural
observation about where the
anchor now sits, and the reason it does not bite may simply be that
`dedupSubqueryOnSelfJoin` rejects the colliding shapes first, per (b). If
you have a query that
reaches the optimizer with the conflict intact, I will add it as a test and
fix what it shows.
**On the missing coverage**, "there is no test here where the nested
relation overlaps either plan"
is now addressed: the new test in `RewriteSubquerySuite` builds that overlap
directly, since the
analyzer will not hand it to us, and pins both the classification and the
error it ends at.
**On the asymmetry inside the helper** — called with the condition from
`buildJoin` and so able to
throw, called without one from the IN arms and so only aliasing — I have
left it as it is. It is
pre-existing, and changing it is the same work as making the colliding
EXISTS shape succeed, which
belongs with a preparation step across all four arms rather than in this
JIRA. Happy to do it here
if you would rather it were not deferred.
--
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]
