pratham76 commented on code in PR #58656:
URL: https://github.com/apache/spark/pull/58656#discussion_r4148301436
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala:
##########
@@ -158,20 +159,23 @@ object RewritePredicateSubquery extends Rule[LogicalPlan]
with PredicateHelper {
// Filter the plan by applying left semi and left anti joins.
withSubquery.foldLeft(newFilter) {
case (p, Exists(sub, _, _, conditions, subHint)) =>
- val (joinCond, outerPlan) = rewriteExistentialExpr(conditions, p)
- val join = buildJoin(outerPlan,
rewriteDomainJoinsIfPresent(outerPlan, sub, joinCond),
+ val (joinCond, outerPlan, newSub) =
+ rewriteExistentialExprInJoinCondition(conditions, p, sub)
+ val join = buildJoin(outerPlan,
rewriteDomainJoinsIfPresent(outerPlan, newSub, joinCond),
LeftSemi, joinCond, subHint)
Project(p.output, join)
case (p, Not(Exists(sub, _, _, conditions, subHint))) =>
- val (joinCond, outerPlan) = rewriteExistentialExpr(conditions, p)
- val join = buildJoin(outerPlan,
rewriteDomainJoinsIfPresent(outerPlan, sub, joinCond),
+ val (joinCond, outerPlan, newSub) =
+ rewriteExistentialExprInJoinCondition(conditions, p, sub)
+ val join = buildJoin(outerPlan,
rewriteDomainJoinsIfPresent(outerPlan, newSub, joinCond),
LeftAnti, joinCond, subHint)
Project(p.output, join)
case (p, InSubquery(values, ListQuery(sub, _, _, _, conditions,
subHint))) =>
// Deduplicate conflicting attributes if any.
- val newSub = dedupSubqueryOnSelfJoin(p, sub, Some(values))
- val inConditions = values.zip(newSub.output).map(EqualTo.tupled)
- val (joinCond, outerPlan) = rewriteExistentialExpr(inConditions ++
conditions, p)
+ val dedupSub = dedupSubqueryOnSelfJoin(p, sub, Some(values))
+ val inConditions = values.zip(dedupSub.output).map(EqualTo.tupled)
+ val (joinCond, outerPlan, newSub) =
+ rewriteExistentialExprInJoinCondition(inConditions ++ conditions,
p, dedupSub)
Review Comment:
The premise checks out. `SizeInBytesOnlyStatsPlanVisitor.visitJoin` gives
`LeftAnti | LeftSemi` an
explicit left-preserving estimate and falls through to `default(p)` for
everything else, so an
`ExistenceJoin` gets the generic product estimate even though it emits
exactly one row per left row
with one Boolean appended. `JoinEstimation` has no `ExistenceJoin` case
either.
Two things I would like to settle before changing it, though.
It is not introduced here. `visitJoin` returns `p.left.stats` for
`LeftSemi`, so an existence join
built on the outer plan — where every nested subquery went before this PR —
already propagated its
inflated estimate through the enclosing semi join. This PR changes which
side carries it, which is
the side broadcast eligibility is decided on, so the consequence you
describe is new even though the
misestimate is not.
And the fix reaches further than this rule. Eight approved TPC-DS plans
contain an `ExistenceJoin`
(q10, q35, q45 and their sf100 variants), so correcting the estimate in
either statistics mode can
change those plans, and the golden files would have to be regenerated and
reviewed. That is a
different kind of change from the routing fix, in a different component,
with its own suites
(`JoinEstimationSuite`, `BasicStatsEstimationSuite`) and its own risk of
moving estimates for plans
that are fine today.
Given that, I would rather do it as a separate JIRA, with your success
criteria carried over
verbatim: a left-derived size in the size-only visitor that includes the
appended Boolean width, an
explicit left-preserving estimate with left-side attribute statistics in the
CBO estimator, and a
regression pinning that an `ExistenceJoin`-wrapped build side stays
broadcast-eligible. Please do let know if that is acceptable? Thanks!
--
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]
