cloud-fan commented on code in PR #43449:
URL: https://github.com/apache/spark/pull/43449#discussion_r1384852516
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -277,28 +269,33 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] with
PredicateHelper with J
leftKeys.lazyZip(rightKeys).foreach((l, r) => {
// Check if:
// 1. There is already a DPP filter on the key
- // 2. There is already a bloom filter on the key
- // 3. The keys are simple cheap expressions
+ // 2. The keys are simple cheap expressions
if (filterCounter < numFilterThreshold &&
!hasDynamicPruningSubquery(left, right, l, r) &&
- !hasBloomFilter(newLeft, newRight, l, r) &&
isSimpleExpression(l) && isSimpleExpression(r)) {
val oldLeft = newLeft
val oldRight = newRight
- // Check if the current join is a shuffle join or a broadcast join
that
- // has a shuffle below it
+ // Check if:
+ // 1. The current join type supports prune the left side with
runtime filter
+ // 2. The current join is a shuffle join or a broadcast join that
+ // has a shuffle below it
+ // 3. There is already a bloom filter on the left key
Review Comment:
```suggestion
// 3. There is no bloom filter on the left key yet
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -277,28 +269,33 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] with
PredicateHelper with J
leftKeys.lazyZip(rightKeys).foreach((l, r) => {
// Check if:
// 1. There is already a DPP filter on the key
- // 2. There is already a bloom filter on the key
- // 3. The keys are simple cheap expressions
+ // 2. The keys are simple cheap expressions
if (filterCounter < numFilterThreshold &&
!hasDynamicPruningSubquery(left, right, l, r) &&
- !hasBloomFilter(newLeft, newRight, l, r) &&
isSimpleExpression(l) && isSimpleExpression(r)) {
val oldLeft = newLeft
val oldRight = newRight
- // Check if the current join is a shuffle join or a broadcast join
that
- // has a shuffle below it
+ // Check if:
+ // 1. The current join type supports prune the left side with
runtime filter
+ // 2. The current join is a shuffle join or a broadcast join that
+ // has a shuffle below it
+ // 3. There is already a bloom filter on the left key
val hasShuffle = isProbablyShuffleJoin(left, right, hint)
- if (canPruneLeft(joinType) && (hasShuffle ||
probablyHasShuffle(left))) {
+ if (canPruneLeft(joinType) && (hasShuffle ||
probablyHasShuffle(left)) &&
+ !hasBloomFilter(newLeft, l)) {
extractBeneficialFilterCreatePlan(left, right, l, r).foreach {
case (filterCreationSideKey, filterCreationSidePlan) =>
newLeft = injectFilter(l, newLeft, filterCreationSideKey,
filterCreationSidePlan)
}
}
// Did we actually inject on the left? If not, try on the right
- // Check if the current join is a shuffle join or a broadcast join
that
- // has a shuffle below it
+ // Check if:
+ // 1. The current join type supports prune the right side with
runtime filter
+ // 2. The current join is a shuffle join or a broadcast join that
+ // has a shuffle below it
+ // 3. There is already a bloom filter on the right key
Review Comment:
ditto
--
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]
