pan3793 commented on code in PR #58635:
URL: https://github.com/apache/spark/pull/58635#discussion_r3985777308


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -411,55 +445,136 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] 
with PredicateHelper with J
   private def tryInjectRuntimeFilter(plan: LogicalPlan): LogicalPlan = {
     var filterCounter = 0
     val numFilterThreshold = 
conf.getConf(SQLConf.RUNTIME_FILTER_NUMBER_THRESHOLD)
+    val bloomFilterEnabled = conf.runtimeFilterBloomFilterEnabled
     plan transformUp {
       case join @ ExtractEquiJoinKeys(joinType, leftKeys, rightKeys, _, _, 
left, right, hint) =>
         var newLeft = left
         var newRight = right
+        // A side hinted as the runtime filter source is the creation side, so 
the filter is
+        // applied to the other side. An ambiguous hint is reported and 
otherwise ignored, leaving
+        // the heuristics to decide.
+        val hintedSource = runtimeFilterSourceSide(hint)
+        val hinted = hintedSource.isDefined
+        val injectLeftHinted = hintedSource.contains(BuildRight)
+        val injectRightHinted = hintedSource.contains(BuildLeft)
+        if (isRuntimeFilterHintAmbiguous(hint)) {
+          hintErrorHandler.joinHintNotSupported(HintInfo(runtimeFilterSource = 
true),
+            "the runtime filter source is ambiguous as both join sides are 
hinted")
+        }
+        var appliedHint = false
+        // The first reason the hint could not be applied on a key. The hinted 
side is the same
+        // for every key, so the first reason is as representative as any.
+        var notAppliedReason: Option[String] = None
+        def hintBlocked(reason: => String): Unit = {
+          if (notAppliedReason.isEmpty) notAppliedReason = Some(reason)
+        }
+        lazy val hasShuffle = isProbablyShuffleJoin(left, right, hint)
+        // Tries to filter `applicationSide` with a filter built from 
`creationSide`. Returns the
+        // filtered side, recording the reason when this direction is the 
hinted one and no filter
+        // was added. Requirements:
+        // 1. The join type supports pruning the application side
+        // 2. The application side is not the hinted source, which is never 
itself filtered
+        // 3. The join is a shuffle join, or a broadcast join with a shuffle 
below it -- an
+        //    estimate of whether the filter pays off, so a hint waives it
+        // 4. There is no Bloom filter on the application side's key yet
+        def tryInject(
+            applicationSide: LogicalPlan,
+            currentApplicationSide: LogicalPlan,
+            applicationSideKey: Expression,
+            creationSide: LogicalPlan,
+            creationSideKey: Expression,
+            canPrune: Boolean,
+            applicationHinted: Boolean,
+            creationHinted: Boolean,
+            sideName: String): Option[LogicalPlan] = {
+          def blocked(reason: => String): Option[LogicalPlan] = {
+            if (applicationHinted) hintBlocked(reason)
+            None
+          }
+          if (!canPrune) {
+            blocked(s"the $sideName side of a " +
+              s"${joinType.sql.toLowerCase(Locale.ROOT)} join cannot be 
pruned")
+          } else if (creationHinted ||

Review Comment:
   Documented in 4959171c248: "A hint that cannot be applied does not make 
Spark build a filter in the opposite direction instead." Pinned by a test where 
the heuristics would filter `t` from the selective `bf2`, and hinting `t` (an 
`ORDER BY ... LIMIT` over a table, not repeatable) yields no filter and the 
warning.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/InjectRuntimeFilter.scala:
##########
@@ -411,55 +445,136 @@ object InjectRuntimeFilter extends Rule[LogicalPlan] 
with PredicateHelper with J
   private def tryInjectRuntimeFilter(plan: LogicalPlan): LogicalPlan = {
     var filterCounter = 0
     val numFilterThreshold = 
conf.getConf(SQLConf.RUNTIME_FILTER_NUMBER_THRESHOLD)
+    val bloomFilterEnabled = conf.runtimeFilterBloomFilterEnabled
     plan transformUp {
       case join @ ExtractEquiJoinKeys(joinType, leftKeys, rightKeys, _, _, 
left, right, hint) =>
         var newLeft = left
         var newRight = right
+        // A side hinted as the runtime filter source is the creation side, so 
the filter is
+        // applied to the other side. An ambiguous hint is reported and 
otherwise ignored, leaving
+        // the heuristics to decide.
+        val hintedSource = runtimeFilterSourceSide(hint)
+        val hinted = hintedSource.isDefined
+        val injectLeftHinted = hintedSource.contains(BuildRight)
+        val injectRightHinted = hintedSource.contains(BuildLeft)
+        if (isRuntimeFilterHintAmbiguous(hint)) {
+          hintErrorHandler.joinHintNotSupported(HintInfo(runtimeFilterSource = 
true),

Review Comment:
   Relation names are gone by optimizer time, and passing the side's own 
`HintInfo` would print the strategy facet in a runtime-filter message. In 
4959171c248 every not-applied warning goes through one helper that appends the 
hinted side and the join condition, e.g. `(hinted side: right, join condition: 
(c1#5 = c2#9))`, which distinguishes several hinted joins.



##########
sql/core/src/test/scala/org/apache/spark/sql/InjectRuntimeFilterSuite.scala:
##########
@@ -260,8 +263,10 @@ class InjectRuntimeFilterSuite extends SharedSparkSession
       case Filter(condition, _) => condition.collect {
         case subquery: org.apache.spark.sql.catalyst.expressions.ScalarSubquery
         => subquery.plan.collect {
+          // A hinted creation side can carry its own `Aggregate` (e.g. a 
`SELECT DISTINCT` one),
+          // so count the Bloom filter aggregates rather than assuming every 
aggregate is one.
           case Aggregate(_, aggregateExpressions, _, _) =>
-            aggregateExpressions.map {
+            aggregateExpressions.collect {

Review Comment:
   Kept the outermost-aggregate rule: an unhinted creation side never contains 
an aggregate, since the extractor stops at one, so it is exactly as strict as 
before for every unhinted plan, and the aggregate/`might_contain` cross-check 
remains. In 4959171c248 the helper also ignores a subquery whose root has no 
Bloom filter aggregate, which is the query's own subquery.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveHints.scala:
##########


Review Comment:
   Renamed to `ResolveJoinHints` in 4959171c248 (`Analyzer`, `ResolveHints`, 
`RuleIdCollection`).



-- 
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]

Reply via email to