peter-toth commented on code in PR #59916:
URL: https://github.com/apache/spark/pull/59916#discussion_r4436023794
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -415,12 +424,15 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] with
PredicateHelper with J
case join @ ExtractEquiJoinKeys(joinType, leftKeys, rightKeys, _, _,
left, right, hint) =>
var newLeft = left
var newRight = right
+ // The side the join is likely to broadcast, picked as `JoinSelection`
picks it
Review Comment:
The comment on `broadcastablePlans` now says that it ignores the other join
hints, and the description lists them as the first known limit. Modelling them
would mean following every condition of `JoinSelection`, and a later change
there could put a bloom filter next to a kept DPP filter. Such a key keeps
getting no runtime filter, as on master.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -415,12 +424,15 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] with
PredicateHelper with J
case join @ ExtractEquiJoinKeys(joinType, leftKeys, rightKeys, _, _,
left, right, hint) =>
var newLeft = left
var newRight = right
+ // The side the join is likely to broadcast, picked as `JoinSelection`
picks it
+ val broadcastSide = getBroadcastBuildSide(join, hintOnly = true, conf)
+ .orElse(getBroadcastBuildSide(join, hintOnly = false, conf))
Review Comment:
Covered now. "do not add bloom filter if the dpp filter reuses the broadcast
of the join" also runs a query without a hint and with
`autoBroadcastJoinThreshold=5000`, so the join broadcasts `bf2` by size.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -380,18 +380,27 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] with
PredicateHelper with J
}
// This checks if there is already a DPP filter, as this rule is called just
after DPP.
+ // `broadcastSide` is the side the join is likely to broadcast. A DPP filter
that runs only by
+ // reusing a broadcast counts only if physical planning is likely to find
one, which needs both:
+ // - exchange reuse is on;
+ // - `broadcastSide` is the filter's build side, or the filter has a
broadcast value projection.
@tailrec
private def hasDynamicPruningSubquery(
left: LogicalPlan,
right: LogicalPlan,
leftKey: Expression,
- rightKey: Expression): Boolean = {
+ rightKey: Expression,
+ broadcastSide: Option[BuildSide]): Boolean = {
+ def isKept(dpp: DynamicPruningSubquery, buildSide: BuildSide): Boolean =
+ !dpp.onlyInBroadcast || (conf.exchangeReuseEnabled &&
+ (broadcastSide.contains(buildSide) ||
dpp.usableBroadcastValueProjection.isDefined))
(left, right) match {
- case (Filter(DynamicPruningSubquery(pruningKey, _, _, _, _, _, _),
plan), _) =>
- pruningKey.fastEquals(leftKey) || hasDynamicPruningSubquery(plan,
right, leftKey, rightKey)
- case (_, Filter(DynamicPruningSubquery(pruningKey, _, _, _, _, _, _),
plan)) =>
- pruningKey.fastEquals(rightKey) ||
- hasDynamicPruningSubquery(left, plan, leftKey, rightKey)
+ case (Filter(dpp: DynamicPruningSubquery, plan), _) =>
+ (dpp.pruningKey.fastEquals(leftKey) && isKept(dpp, BuildRight)) ||
+ hasDynamicPruningSubquery(plan, right, leftKey, rightKey,
broadcastSide)
+ case (_, Filter(dpp: DynamicPruningSubquery, plan)) =>
+ (dpp.pruningKey.fastEquals(rightKey) && isKept(dpp, BuildLeft)) ||
Review Comment:
Covered now. "add bloom filter if the dpp filter needs a broadcast that the
shuffle join lacks" and "do not add bloom filter if the dpp filter reuses the
broadcast of the join" both run with the partitioned table on either side of
the join.
--
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]
