sunchao commented on code in PR #56603:
URL: https://github.com/apache/spark/pull/56603#discussion_r3444290603


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/dynamicpruning/PartitionPruning.scala:
##########
@@ -134,45 +134,58 @@ object PartitionPruning extends Rule[LogicalPlan] with 
PredicateHelper with Join
    * in bytes of the plan on the other side of the join. We estimate the 
filtering ratio
    * using column statistics if they are available, otherwise we use the 
config value of
    * `spark.sql.optimizer.dynamicPartitionPruning.fallbackFilterRatio`.
+   *
+   * The fallback ratio is only meaningful "when CBO stats are missing, but 
there is a predicate
+   * that is likely to be selective" -- so it is used only when 
`hasSelectivePredicate` is true. A
+   * filtering side that is eligible only because it is already materialized 
(a LocalRelation or a
+   * checkpoint-derived LogicalRDD, SPARK-54593) carries no such predicate; 
for it we rely solely on
+   * the statistics-based ratio and report no benefit when statistics are 
unavailable, so it is not
+   * injected as a standalone always-applied subquery on a guessed ratio. A 
statistics-based ratio,
+   * when available, is always honored regardless of `hasSelectivePredicate`.
    */
   private def pruningHasBenefit(
       partExpr: Expression,
       partPlan: LogicalPlan,
       otherExpr: Expression,
-      otherPlan: LogicalPlan): Boolean = {
+      otherPlan: LogicalPlan,
+      hasSelectivePredicate: Boolean): Boolean = {
 
     // get the distinct counts of an attribute for a given table
     def distinctCounts(attr: Attribute, plan: LogicalPlan): Option[BigInt] = {
       plan.stats.attributeStats.get(attr).flatMap(_.distinctCount)
     }
 
-    // the default filtering ratio when CBO stats are missing, but there is a
-    // predicate that is likely to be selective
-    val fallbackRatio = conf.dynamicPartitionPruningFallbackFilterRatio
-    // the filtering ratio based on the type of the join condition and on the 
column statistics
-    val filterRatio = (partExpr.references.toList, 
otherExpr.references.toList) match {
-      // filter out expressions with more than one attribute on any side of 
the operator
-      case (leftAttr :: Nil, rightAttr :: Nil)
-        if conf.dynamicPartitionPruningUseStats =>
-          // get the CBO stats for each attribute in the join condition
-          val partDistinctCount = distinctCounts(leftAttr, partPlan)
-          val otherDistinctCount = distinctCounts(rightAttr, otherPlan)
-          val availableStats = partDistinctCount.isDefined && 
partDistinctCount.get > 0 &&
-            otherDistinctCount.isDefined
-          if (!availableStats) {
-            fallbackRatio
-          } else if (partDistinctCount.get.toDouble <= 
otherDistinctCount.get.toDouble) {
-            // there is likely an estimation error, so we fallback
-            fallbackRatio
-          } else {
-            1 - otherDistinctCount.get.toDouble / 
partDistinctCount.get.toDouble
-          }
-      case _ => fallbackRatio
+    // the filtering ratio derived from column statistics, when reliable stats 
are available
+    val statsBasedRatio: Option[Double] =
+      (partExpr.references.toList, otherExpr.references.toList) match {
+        // filter out expressions with more than one attribute on any side of 
the operator
+        case (leftAttr :: Nil, rightAttr :: Nil)
+          if conf.dynamicPartitionPruningUseStats =>
+            // get the CBO stats for each attribute in the join condition
+            val partDistinctCount = distinctCounts(leftAttr, partPlan)
+            val otherDistinctCount = distinctCounts(rightAttr, otherPlan)
+            val availableStats = partDistinctCount.isDefined && 
partDistinctCount.get > 0 &&
+              otherDistinctCount.isDefined
+            if (!availableStats) {
+              None

Review Comment:
   [P2] Preserve standalone DPP when an exact row bound proves selectivity
   
   This `None` path also covers a `LocalRelation`, even though its `maxRows` is 
exact and provides a conservative upper bound for the join-key NDV. For 
example, with a one-row `LocalRelation` and a fact table whose partition key 
has NDV 20, `otherNDV <= 1`, so the estimated pruning ratio is at least `1 - 
1/20 = 95%`. In a focused parent-versus-head run with 
`reuseBroadcastOnly=false`, broadcast joins disabled, and no reusable 
broadcast, the parent inserted standalone DPP and scanned 1 of 20 partitions; 
this head inserted no DPP and scanned all 20, while returning identical 
results. For the same shape at larger scale, selecting one day from a 365-day 
partitioned table can turn a one-partition scan into a 365-partition scan.
   
   Could we use `otherPlan.maxRows` as a conservative NDV upper bound when 
column statistics are absent, and then apply the existing overhead comparison? 
Otherwise this is a demonstrated beneficial standalone-DPP case rather than a 
no-evidence case.



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