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:
You are right about the asymmetry, and about its effect. I could not reach
it from SQL, because
`DeduplicateRelations` removes the overlap from every analyzed query I
tried, so I built the state
directly as a plan in `RewriteSubquerySuite`, with `a` as the same attribute
in the outer and in the
subquery plan. The nested subquery, which references only the subquery it is
nested in, is then
rejected with `NESTED_SUBQUERY_REFERENCING_OUTER_AND_INNER_QUERY` — the
misdescribed rejection you
predicted.
I did not deduplicate `sub` before the call, though, because for the EXISTS
arms that is not
equivalent to what the IN arms do. `dedupSubqueryOnSelfJoin` aliases the
subquery plan's `a` to a
fresh ExprId, while the hoisted `conditions` still say `a`; the semi join's
left side is the outer
plan, which still produces `a`, so the condition would rebind to the outer
attribute — a correlated
predicate silently turned into a self-reference, with no error. The IN arms
are safe from that only
because they rebuild `inConditions` from `dedupSub.output`; the hoisted
`conditions` they pass
through have the same exposure.
What I changed instead is the classification, so that it behaves as though
the subplan had been
deduplicated. `effectivelyReferencesPlanOnly` attributes an attribute that
*both* plans produce to
the subquery plan, on the grounds that a nested subquery resolves in the
scope of the subquery it
sits in, so a shared ExprId is that subquery's own. Only an attribute the
outer plan alone produces
now counts as an outer reference, which is what the IN arms effectively get
from their deduplicated
plan.
One thing that came out of testing it, relevant to your point (b) in the
dedup-anchor comment: in
that state the rewrite cannot finish either way. `buildJoin` hands the semi
join condition to
`dedupSubqueryOnSelfJoin`, which has always rejected a condition naming a
duplicated attribute, so
the query ends at `_LEGACY_ERROR_TEMP_1212`. A correlated EXISTS's hoisted
condition always names
subplan attributes, so any shared ExprId it mentions lands there. The
routing fix therefore changes
which error the user sees — from a 0A000 that misdescribes the query to the
long-standing conflict
error that names the duplicated attribute — rather than making the query
succeed. The new test
asserts the conflict error and fails with the 0A000 if the classification is
reverted.
--
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]
