LuciferYang commented on code in PR #55579:
URL: https://github.com/apache/spark/pull/55579#discussion_r4173544005


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileScan.scala:
##########
@@ -182,8 +185,80 @@ trait FileScan extends Scan
     FilePartition.getFilePartitions(sparkSession, splitFiles, maxSplitBytes)
   }
 
+  /**
+   * The partitions `partitions` planned, computed once per scan instance. 
Both the plain read path
+   * and the runtime-filter path go through this, so a scan node that gets 
runtime filters lists the
+   * files once and filters that listing, rather than reading the index twice 
and risking two
+   * snapshots. A subclass still customizes `partitions`.
+   */
+  @transient private lazy val plannedPartitions: Seq[FilePartition] = 
partitions
+
   override def planInputPartitions(): Array[InputPartition] = {
-    partitions.toArray
+    plannedPartitions.toArray
+  }
+
+  /**
+   * The partition columns Spark can derive a runtime filter on (SPARK-30628), 
restricted to the
+   * ones `readSchema()` still exposes: a reference missing from the scan 
relation output fails to
+   * resolve, and a pushed-down aggregate keeps only the partition columns it 
groups by.
+   *
+   * A filter over one of them is evaluated against each file's partition 
values, so the scan
+   * evaluates it in full and Spark does not evaluate it again after the scan 
--
+   * `FileScanBuilder.pushFilters` already keeps compile-time partition 
filters out of the post-scan
+   * filters for the same reason.
+   */
+  override def filterAttributes(): Array[NamedReference] = {
+    val readFields = readSchema().fieldNames.map(normalizeName).toSet
+    readPartitionSchema.fieldNames
+      .filter(name => readFields.contains(normalizeName(name)))
+      .map(FieldReference.column)
+  }
+
+  override def fullyPushedFilterAttributes(): Array[NamedReference] = 
filterAttributes()
+
+  /**
+   * Plans the files whose partition values satisfy `expressions`, taken from 
what `partitions`
+   * planned and bin-packed again for the surviving set.
+   *
+   * Filtering what was planned rather than listing the files again keeps one 
listing per scan, and
+   * the files a subclass excluded in `partitions` stay excluded. How the 
survivors group into
+   * partitions is decided here again, for their own size. Where each file was 
cut into splits is
+   * not: `partitions` cut them for the whole set it listed, before these 
filters existed.
+   *
+   * A partition value is fixed within a file, so every row of every partition 
returned satisfies
+   * the expressions even though one partition can still hold files from 
several partition
+   * directories.
+   */
+  override def planInputPartitionsWithRuntimeFilters(
+      expressions: Array[Expression]): Array[InputPartition] = {
+    val fieldIndex = readPartitionSchema.fieldNames.zipWithIndex
+      .map { case (name, i) => normalizeName(name) -> i }.toMap
+    // Evaluating these is the scan's only chance to apply them, since the 
attributes are declared
+    // fully pushed and Spark drops the post-scan filter. A reference outside 
the read partition
+    // schema has no value to evaluate against, so fail loudly rather than 
return wrong rows. Spark
+    // screens against `filterAttributes()` before it gets here, which is a 
subset of that schema.
+    val notApplicable = expressions.filterNot(
+      _.references.forall(a => fieldIndex.contains(normalizeName(a.name))))
+    if (notApplicable.nonEmpty) {
+      throw SparkException.internalError("A file scan can only apply a runtime 
filter over its " +
+        s"read partition columns 
${readPartitionSchema.fieldNames.mkString("[", ", ", "]")}, " +
+        s"got ${notApplicable.mkString(", ")}")
+    }
+    val bound = expressions.reduce(And).transform {
+      case a: Attribute => BoundReference(fieldIndex(normalizeName(a.name)), 
a.dataType, a.nullable)
+    }
+    val predicate = Predicate.createInterpreted(bound)
+    val keptFiles = plannedPartitions

Review Comment:
   This and item 2 come down to per-format opt-in versus an overridable hook. I 
compared the two against downstream-shaped scans in 
https://github.com/apache/spark/pull/55579#issuecomment-5966973332 and will 
change this once we agree on one.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileScan.scala:
##########
@@ -43,6 +45,7 @@ trait FileScan extends Scan
   with Batch
   with SupportsReportStatistics
   with SupportsMetadata
+  with SupportsRuntimeCatalystFiltering

Review Comment:
   Same as item 1. The comparison is in 
https://github.com/apache/spark/pull/55579#issuecomment-5966973332, along with 
a question for pan3793 about how Kyuubi would move.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/PushDownUtils.scala:
##########
@@ -230,48 +231,64 @@ object PushDownUtils extends Logging {
 
         translatedFiltersPushed || partPredicatesPushed
 
-      case catalystScan: SupportsRuntimeCatalystFiltering if 
pushableFilters.nonEmpty =>
-        // A runtime filter is normally evaluated twice: the source prunes 
with it, and the
-        // FilterExec above the scan applies it again. The two have to agree, 
so this screen
-        // pushes only predicates the source can be trusted to evaluate in 
Spark's place.
-        //
-        // But pushing a non-deterministic one would let the source prune on 
its own coin flip and
-        // Spark flip again for the rows that survive, so we handle that 
specially.
-        //
-        // Not pushing a filter is safe only while its FilterExec still 
evaluates it. A fully
-        // pushed filter has none, so the source is its only evaluator. That 
is why
-        // DataSourceV2Strategy deletes a FilterExec at planning time only for 
a filter we push
-        // below, and we use the same method (isPushablePartitionFilter) to 
determine if we
-        // should push it.
-        //
-        // A DPP filter degrades to TrueLiteral once its subquery is pruned 
away, so it matches
-        // every row. The V2 path above drops these implicitly, since 
translateRuntimeFilterV2
-        // returns None; here we push Catalyst expressions directly, so we 
remove them explicitly.
-        val catalystFilters = pushableFilters
-          .flatMap(unwrapRuntimeFilterExpression)
-          .filterNot(_ == Literal.TrueLiteral)
-        if (catalystFilters.nonEmpty) {
-          catalystScan.filter(catalystFilters.toArray)
-          true
-        } else {
-          false
-        }
-
       case _ =>

Review Comment:
   Done in 2acbef9. `pushRuntimeFilters` now throws an internal error pointing 
to `replanWithRuntimeFilters` when a `SupportsRuntimeCatalystFiltering` scan 
reaches it with a pushable filter. With nothing to push it still returns false, 
which is the call `replanWithRuntimeFilters` makes for a Catalyst scan with no 
runtime filters, and the scaladoc now says so. `Catalyst scan handed to 
pushRuntimeFilters with a pushable filter -> rejected` covers it.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileScan.scala:
##########
@@ -182,8 +185,80 @@ trait FileScan extends Scan
     FilePartition.getFilePartitions(sparkSession, splitFiles, maxSplitBytes)
   }
 
+  /**
+   * The partitions `partitions` planned, computed once per scan instance. 
Both the plain read path
+   * and the runtime-filter path go through this, so a scan node that gets 
runtime filters lists the
+   * files once and filters that listing, rather than reading the index twice 
and risking two
+   * snapshots. A subclass still customizes `partitions`.
+   */
+  @transient private lazy val plannedPartitions: Seq[FilePartition] = 
partitions
+
   override def planInputPartitions(): Array[InputPartition] = {
-    partitions.toArray
+    plannedPartitions.toArray
+  }
+
+  /**
+   * The partition columns Spark can derive a runtime filter on (SPARK-30628), 
restricted to the
+   * ones `readSchema()` still exposes: a reference missing from the scan 
relation output fails to
+   * resolve, and a pushed-down aggregate keeps only the partition columns it 
groups by.
+   *
+   * A filter over one of them is evaluated against each file's partition 
values, so the scan
+   * evaluates it in full and Spark does not evaluate it again after the scan 
--
+   * `FileScanBuilder.pushFilters` already keeps compile-time partition 
filters out of the post-scan
+   * filters for the same reason.
+   */
+  override def filterAttributes(): Array[NamedReference] = {
+    val readFields = readSchema().fieldNames.map(normalizeName).toSet
+    readPartitionSchema.fieldNames
+      .filter(name => readFields.contains(normalizeName(name)))
+      .map(FieldReference.column)
+  }
+
+  override def fullyPushedFilterAttributes(): Array[NamedReference] = 
filterAttributes()
+
+  /**
+   * Plans the files whose partition values satisfy `expressions`, taken from 
what `partitions`
+   * planned and bin-packed again for the surviving set.
+   *
+   * Filtering what was planned rather than listing the files again keeps one 
listing per scan, and
+   * the files a subclass excluded in `partitions` stay excluded. How the 
survivors group into
+   * partitions is decided here again, for their own size. Where each file was 
cut into splits is
+   * not: `partitions` cut them for the whole set it listed, before these 
filters existed.
+   *
+   * A partition value is fixed within a file, so every row of every partition 
returned satisfies
+   * the expressions even though one partition can still hold files from 
several partition
+   * directories.
+   */
+  override def planInputPartitionsWithRuntimeFilters(
+      expressions: Array[Expression]): Array[InputPartition] = {
+    val fieldIndex = readPartitionSchema.fieldNames.zipWithIndex
+      .map { case (name, i) => normalizeName(name) -> i }.toMap
+    // Evaluating these is the scan's only chance to apply them, since the 
attributes are declared
+    // fully pushed and Spark drops the post-scan filter. A reference outside 
the read partition
+    // schema has no value to evaluate against, so fail loudly rather than 
return wrong rows. Spark
+    // screens against `filterAttributes()` before it gets here, which is a 
subset of that schema.
+    val notApplicable = expressions.filterNot(
+      _.references.forall(a => fieldIndex.contains(normalizeName(a.name))))
+    if (notApplicable.nonEmpty) {
+      throw SparkException.internalError("A file scan can only apply a runtime 
filter over its " +
+        s"read partition columns 
${readPartitionSchema.fieldNames.mkString("[", ", ", "]")}, " +
+        s"got ${notApplicable.mkString(", ")}")
+    }
+    val bound = expressions.reduce(And).transform {
+      case a: Attribute => BoundReference(fieldIndex(normalizeName(a.name)), 
a.dataType, a.nullable)
+    }
+    val predicate = Predicate.createInterpreted(bound)
+    val keptFiles = plannedPartitions
+      .flatMap(_.files.filter(file => predicate.eval(file.partitionValues)))
+      .sortBy(_.length)(implicitly[Ordering[Long]].reverse)
+    // Pack the survivors instead of handing back the planned partitions with 
files removed:
+    // `partitions` sized those bins for the whole file set, so a scan that 
pruned most of its files
+    // would run the same few tasks over a fraction of the data. V1 packs the 
pruned set the same
+    // way, from `dynamicallySelectedPartitions`.
+    val openCostInBytes = conf.filesOpenCostInBytes
+    val maxSplitBytes = FilePartition.maxSplitBytes(
+      sparkSession, keptFiles.map(_.length + openCostInBytes).sum)
+    FilePartition.getFilePartitions(sparkSession, keptFiles, 
maxSplitBytes).toArray

Review Comment:
   Agreed, the repack keeps only the file set. Whether to gate it on reported 
partitioning or ordering depends on items 1 and 2: with per-format opt-in only 
the built-in scans take this path, and none of them reports either. In 2acbef9 
the test comment now claims only that a file the override drops stays dropped. 
I'll fix the description's wording when I next update it.



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/FileScan.scala:
##########
@@ -182,8 +185,80 @@ trait FileScan extends Scan
     FilePartition.getFilePartitions(sparkSession, splitFiles, maxSplitBytes)
   }
 
+  /**
+   * The partitions `partitions` planned, computed once per scan instance. 
Both the plain read path
+   * and the runtime-filter path go through this, so a scan node that gets 
runtime filters lists the
+   * files once and filters that listing, rather than reading the index twice 
and risking two
+   * snapshots. A subclass still customizes `partitions`.
+   */
+  @transient private lazy val plannedPartitions: Seq[FilePartition] = 
partitions
+
   override def planInputPartitions(): Array[InputPartition] = {
-    partitions.toArray
+    plannedPartitions.toArray
+  }
+
+  /**
+   * The partition columns Spark can derive a runtime filter on (SPARK-30628), 
restricted to the
+   * ones `readSchema()` still exposes: a reference missing from the scan 
relation output fails to
+   * resolve, and a pushed-down aggregate keeps only the partition columns it 
groups by.
+   *
+   * A filter over one of them is evaluated against each file's partition 
values, so the scan
+   * evaluates it in full and Spark does not evaluate it again after the scan 
--
+   * `FileScanBuilder.pushFilters` already keeps compile-time partition 
filters out of the post-scan
+   * filters for the same reason.
+   */
+  override def filterAttributes(): Array[NamedReference] = {
+    val readFields = readSchema().fieldNames.map(normalizeName).toSet
+    readPartitionSchema.fieldNames
+      .filter(name => readFields.contains(normalizeName(name)))
+      .map(FieldReference.column)
+  }
+
+  override def fullyPushedFilterAttributes(): Array[NamedReference] = 
filterAttributes()

Review Comment:
   Done in 2acbef9. `BatchScanExec` overrides `verboseStringWithOperatorId` to 
add a redacted `RuntimeFilters: [...]` line when it has runtime filters. A new 
`ExplainSuite` case checks that line for a scalar subquery on a V2 parquet 
partition column, and checks that no `Filter` node is left.



##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2FileSourceDPPSuite.scala:
##########
@@ -0,0 +1,658 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connector
+
+import java.io.File
+
+import org.apache.spark.{SparkException, SparkThrowable}
+import org.apache.spark.sql.{DataFrame, QueryTest, Row}
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, 
DynamicPruningExpression, DynamicPruningSubquery, EqualTo, Expression, Literal}
+import org.apache.spark.sql.catalyst.plans.logical.{Filter, LogicalPlan}
+import org.apache.spark.sql.execution.FilterExec
+import org.apache.spark.sql.execution.adaptive.{AdaptiveSparkPlanHelper, 
DisableAdaptiveExecutionSuite, EnableAdaptiveExecutionSuite}
+import org.apache.spark.sql.execution.datasources.FilePartition
+import org.apache.spark.sql.execution.datasources.v2.{BatchScanExec, FileScan}
+import org.apache.spark.sql.execution.datasources.v2.parquet.ParquetScan
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{IntegerType, LongType}
+
+/**
+ * Tests for SPARK-30628: subquery partition pruning and DPP for V2 file 
sources.
+ *
+ * These tests exercise V2 file scans (e.g. ParquetScan) -- distinct from the 
in-memory V2
+ * catalog used by [[org.apache.spark.sql.DynamicPartitionPruningV2Suite]] and 
from the V2
+ * iterative-pushdown work covered by 
[[DataSourceV2EnhancedRuntimePartitionFilterSuite]].
+ */
+abstract class DataSourceV2FileSourceDPPSuiteBase extends QueryTest
+  with SharedSparkSession
+  with AdaptiveSparkPlanHelper {
+
+  // Standard V2-only conf for DPP tests on file sources. AQE is set by the 
concrete subclasses,
+  // which mix in DisableAdaptiveExecutionSuite / 
EnableAdaptiveExecutionSuite: the DPP subquery is
+  // planned by PlanDynamicPruningFilters without AQE and by 
PlanAdaptiveDynamicPruningFilters with
+  // it, and those are two separate rules.
+  private def withDppV2Conf[T](thunk: => T): T = {
+    withSQLConf(
+      SQLConf.USE_V1_SOURCE_LIST.key -> "",
+      SQLConf.DYNAMIC_PARTITION_PRUNING_ENABLED.key -> "true",
+      SQLConf.DYNAMIC_PARTITION_PRUNING_REUSE_BROADCAST_ONLY.key -> "false",
+      SQLConf.DYNAMIC_PARTITION_PRUNING_FALLBACK_FILTER_RATIO.key -> 
"2")(thunk)
+  }
+
+  // Writes a partitioned `fact` and an unpartitioned `dim` table in `format` 
and registers them
+  // as temp views. `fact` has 100 rows across 10 partitions (10 rows each); 
`dim` has 10 rows
+  // with `dim_id` and `dim_val` both in [0, 10). The read schema is given 
explicitly because csv
+  // infers strings, and a cast on the join key changes whether DPP fires at 
all.
+  private def writeFactAndDim(dir: File, format: String = "parquet"): Unit = {
+    val factPath = new File(dir, "fact").getCanonicalPath
+    val dimPath = new File(dir, "dim").getCanonicalPath
+    spark.range(100)
+      .selectExpr("id", "id % 10 AS part")
+      .write.format(format).partitionBy("part").save(factPath)
+    spark.read.format(format).schema("id long, part int").load(factPath)
+      .createOrReplaceTempView("fact")
+    spark.range(10)
+      .selectExpr("id AS dim_id", "id AS dim_val")
+      .write.format(format).save(dimPath)
+    spark.read.format(format).schema("dim_id long, dim_val long").load(dimPath)
+      .createOrReplaceTempView("dim")
+  }
+
+  // The BatchScanExec reading `fact`, the only partitioned table of the two. 
The lookup descends
+  // into AQE query stages, so it finds the scan whichever stage it ended up 
in.
+  private def factScanOf(df: DataFrame): BatchScanExec = {
+    val executedPlan = df.queryExecution.executedPlan
+    collectFirst(executedPlan) {
+      case b: BatchScanExec if b.scan.isInstanceOf[FileScan] &&
+        b.scan.asInstanceOf[FileScan].readPartitionSchema.nonEmpty => b
+    }.getOrElse(fail("no fact BatchScanExec found in:\n" + 
executedPlan.treeString))
+  }
+
+  private def fileScanOf(df: DataFrame): FileScan = 
factScanOf(df).scan.asInstanceOf[FileScan]
+
+  private def collectDppFilters(plan: LogicalPlan): Seq[Expression] = {
+    plan.collect {
+      case f: Filter if 
f.condition.exists(_.isInstanceOf[DynamicPruningSubquery]) => f.condition
+    }
+  }
+
+  test("DPP inserts DynamicPruningSubquery for V2 parquet partitioned table") {
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val df = sql(
+          """SELECT f.id FROM fact f JOIN dim d
+            |ON f.part = d.dim_id WHERE d.dim_val = 7""".stripMargin)
+        val optimized = df.queryExecution.optimizedPlan
+        assert(collectDppFilters(optimized).nonEmpty,
+          "expected DynamicPruningSubquery in optimized plan over V2 parquet, 
got plan:\n" +
+            optimized.treeString)
+      }
+    }
+  }
+
+  test("DPP does not fire when join key is a data column (non-partition)") {
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        // Join on f.id (data column), not f.part (partition column). `f.part` 
stays in the
+        // projection so the fact scan still reads a partition column: without 
it the scan declares
+        // nothing filterable for a reason that has nothing to do with the 
join key.
+        val df = sql(
+          """SELECT f.id, f.part FROM fact f JOIN dim d
+            |ON f.id = d.dim_id WHERE d.dim_val = 7""".stripMargin)
+        val optimized = df.queryExecution.optimizedPlan
+        assert(collectDppFilters(optimized).isEmpty,
+          "DPP should not fire on non-partition join keys, got plan:\n" +
+            optimized.treeString)
+        assert(df.collect().length === 1)
+        // And the scan is one that could have been pruned, had the join key 
been its partition
+        // column: a filterAttributes() reporting every read column would make 
DPP fire above, and
+        // the fact scan would read fewer than all 100 rows.
+        assert(fileScanOf(df).filterAttributes().flatMap(_.fieldNames).toSeq 
=== Seq("part"))
+        val numOutputRows = factScanOf(df).metrics("numOutputRows").value
+        assert(numOutputRows === 100,
+          s"expected the fact scan to read every row, got $numOutputRows")
+      }
+    }
+  }
+
+  test("DPP fires when partitioned fact is on the right side of the join") {
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        // dim on left, fact on right; join on partition column
+        val df = sql(
+          """SELECT f.id FROM dim d JOIN fact f
+            |ON d.dim_id = f.part WHERE d.dim_val = 7""".stripMargin)
+        val optimized = df.queryExecution.optimizedPlan
+        assert(collectDppFilters(optimized).nonEmpty,
+          "expected DPP on right-side partitioned fact, got plan:\n" +
+            optimized.treeString)
+        checkAnswer(df, (0 until 10).map(i => Row(i * 10 + 7)))
+        val numOutputRows = factScanOf(df).metrics("numOutputRows").value
+        assert(numOutputRows === 10,
+          s"expected the fact scan to read only partition 7, got 
$numOutputRows rows")
+      }
+    }
+  }
+
+  test("DPP filter reaches BatchScanExec.runtimeFilters as 
DynamicPruningExpression") {
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val df = sql(
+          """SELECT f.id FROM fact f JOIN dim d
+            |ON f.part = d.dim_id WHERE d.dim_val = 7""".stripMargin)
+        df.collect()
+        val factScan = factScanOf(df)
+        // A DPP filter that degraded to true is also a 
DynamicPruningExpression, so match on the
+        // payload: only a subquery child means the filter can actually prune 
anything.
+        val dppFilters = factScan.runtimeFilters.collect {
+          case d @ DynamicPruningExpression(child) if 
!child.isInstanceOf[Literal] => d
+        }
+        assert(dppFilters.nonEmpty,
+          "expected the fact scan's runtimeFilters to carry a 
DynamicPruningExpression over a " +
+            s"subquery, got ${factScan.runtimeFilters}")
+      }
+    }
+  }
+
+  test("DPP prunes input partitions at runtime for v2 parquet") {
+    // Fact has 100 rows across 10 partitions (10 rows each). DPP filter 
d.dim_val = 7 selects
+    // exactly partition 7 -> 10 rows. Without runtime pruning, all 100 rows 
are read.
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val df = sql(
+          """SELECT f.id FROM fact f JOIN dim d
+            |ON f.part = d.dim_id WHERE d.dim_val = 7""".stripMargin)
+        val rows = df.collect()
+        // Correctness: 10 fact rows match (partition 7), each multiplied by 1 
dim row.
+        assert(rows.length == 10,
+          s"expected 10 rows after join+filter, got ${rows.length}")
+        // Pruning: fact scan should have read only the 10 rows in partition 7.
+        val numOutputRows = factScanOf(df).metrics("numOutputRows").value
+        assert(numOutputRows == 10,
+          s"expected fact scan to read 10 rows after DPP, got $numOutputRows. 
plan:\n" +
+            df.queryExecution.executedPlan.treeString)
+      }
+    }
+  }
+
+  test("DPP that prunes every partition leaves the scan with no input 
partition") {
+    // `fact5` has partitions 0..4 while the dim row selects 7, so the build 
side is not empty and
+    // the surviving partition set is. That combination is what reaches the 
zero-partition
+    // DataSourceRDD: BatchScanExec's EmptyRDDWithPartitions branch keys off 
SinglePartition, which
+    // a file scan never reports.
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val factPath = new File(dir, "fact5").getCanonicalPath
+        spark.range(50).selectExpr("id", "id % 5 AS part")
+          .write.format("parquet").partitionBy("part").save(factPath)
+        spark.read.format("parquet").schema("id long, part int").load(factPath)
+          .createOrReplaceTempView("fact5")
+        val df = sql(
+          """SELECT f.id FROM fact5 f JOIN dim d
+            |ON f.part = d.dim_id WHERE d.dim_val = 7""".stripMargin)
+        assert(df.collect().isEmpty)
+        val factScan = factScanOf(df)
+        assert(factScan.filteredPartitions.flatten.isEmpty,
+          s"expected no input partition, got 
${factScan.filteredPartitions.flatten.size}")
+        // What the assertion above does not see: BatchScanExec turning an 
empty partition set into
+        // an RDD with one empty partition still satisfies it.
+        assert(factScan.inputRDD.partitions.isEmpty,
+          s"expected an RDD with no partition, got 
${factScan.inputRDD.partitions.length}")
+        val numOutputRows = factScan.metrics("numOutputRows").value
+        assert(numOutputRows == 0,
+          s"expected the fact scan to read nothing, got $numOutputRows rows")
+      }
+    }
+  }
+
+  test("a DPP filter a union branch cannot apply is dropped instead of failing 
the query") {
+    // PartitionPruning resolves a union key through the first branch only, 
and the filter it
+    // inserts is then pushed positionally into every branch. So the 
unpartitioned branch's scan
+    // is handed a filter over a column it never declared filterable. Dropping 
it costs pruning
+    // on that branch and nothing else, since the join re-applies it; letting 
it through means an
+    // INTERNAL_ERROR from a query that runs today.
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val otherPath = new File(dir, "fact_other").getCanonicalPath
+        spark.range(100).selectExpr("id", "id % 10 AS other")
+          .write.format("parquet").save(otherPath)
+        spark.read.format("parquet").schema("id long, other 
long").load(otherPath)
+          .createOrReplaceTempView("fact_other")
+        val df = sql(
+          """SELECT u.id FROM (
+            |  SELECT id, part FROM fact UNION ALL SELECT id, other FROM 
fact_other
+            |) u JOIN dim d ON u.part = d.dim_id WHERE d.dim_val = 
7""".stripMargin)
+        val expected = (0 until 10).map(i => Row(i * 10 + 7))
+        checkAnswer(df, expected ++ expected)
+        // The filter did reach the branch that cannot use it: that is what 
makes this a regression
+        // test rather than a query that happens to work. Both the 
unpartitioned branch and `dim`
+        // are scans with no partition column, so ask for the one that was 
handed a live DPP filter.
+        val unusable = collect(df.queryExecution.executedPlan) {
+          case b: BatchScanExec if b.scan.isInstanceOf[FileScan] &&
+            b.scan.asInstanceOf[FileScan].readPartitionSchema.isEmpty &&
+            b.runtimeFilters.exists {
+              case DynamicPruningExpression(child) => 
!child.isInstanceOf[Literal]
+              case _ => false
+            } => b
+        }
+        assert(unusable.size == 1,
+          "expected the DPP filter to reach exactly the branch that cannot 
apply it, got " +
+            unusable.map(_.scan.description()).mkString("\n"))
+        // And the branch that can use it still pruned.
+        val numOutputRows = factScanOf(df).metrics("numOutputRows").value
+        assert(numOutputRows == 10,
+          s"expected the partitioned branch to read 10 rows, got 
$numOutputRows")
+      }
+    }
+  }
+
+  test("a partition value the runtime filter cannot evaluate fails the query") 
{
+    // A partition value the filter cannot be evaluated against has to fail 
the query: keeping the
+    // file would return rows that nothing filters, since the filter is 
declared fully pushed. What
+    // this pins is that the failure is not swallowed. The exposure is the 
same one a compile-time
+    // partition filter over the same column has, and it is why this path must 
not gain the
+    // eval-failure tolerance the iterative PartitionPredicate path has. That 
no FilterExec is left
+    // above the scan is pinned separately.
+    withDppV2Conf {
+      withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
+        withTempDir { dir =>
+          writeFactAndDim(dir)
+          val strPath = new File(dir, "fact_str").getCanonicalPath
+          spark.range(10)
+            .selectExpr("id", "CASE WHEN id = 3 THEN 'abc' ELSE CAST(id AS 
STRING) END AS part")
+            .write.format("parquet").partitionBy("part").save(strPath)
+          spark.read.format("parquet").schema("id long, part 
string").load(strPath)
+            .createOrReplaceTempView("fact_str")
+          val e = intercept[SparkThrowable] {
+            sql(
+              """SELECT f.id FROM fact_str f
+                |WHERE f.part = (SELECT max(dim_id) FROM dim WHERE dim_val = 
7)""".stripMargin)
+              .collect()
+          }
+          assert(e.getCondition === "CAST_INVALID_INPUT",
+            s"expected the failing cast to surface, got ${e.getCondition}")
+        }
+      }
+    }
+  }
+
+  test("a subclass that narrows the file set in partitions keeps it under a 
runtime filter") {
+    // The runtime-filter path filters what `partitions` returned, so an 
override of it is honored.
+    // Listing the files again instead would hand the excluded ones back as 
soon as a filter fired,
+    // and with the filter declared fully pushed nothing above the scan would 
remove them.
+    withDppV2Conf {
+      withTempDir { dir =>
+        writeFactAndDim(dir)
+        val df = sql("SELECT f.id, f.part FROM fact f")
+        df.collect()
+        val scan = fileScanOf(df).asInstanceOf[ParquetScan]
+        // Keeps the files of one partition directory, which the second filter 
below does not
+        // select, and counts how often the scan asks for its partitions.
+        var partitionsCalls = 0
+        val narrowed = new ParquetScan(scan.sparkSession, scan.hadoopConf, 
scan.fileIndex,
+          scan.dataSchema, scan.readDataSchema, scan.readPartitionSchema, 
scan.pushedFilters,
+          scan.options, scan.pushedAggregate, scan.partitionFilters, 
scan.dataFilters,
+          scan.pushedVariantExtractions) {
+          override protected def partitions: Seq[FilePartition] = {
+            partitionsCalls += 1
+            super.partitions.map { part =>
+              part.copy(files = part.files.filter(_.partitionValues.getInt(0) 
== 3))
+            }.filter(_.files.nonEmpty)
+          }
+        }
+        val partAttr = AttributeReference("part", IntegerType)()
+        val kept = narrowed.planInputPartitionsWithRuntimeFilters(
+          Array(EqualTo(partAttr, Literal(3))))
+        
assert(kept.flatMap(_.asInstanceOf[FilePartition].files.map(_.partitionValues.getInt(0)))
+          .distinct.toSeq === Seq(3))
+        // And a filter selecting a directory the override dropped comes back 
with nothing.
+        assert(narrowed.planInputPartitionsWithRuntimeFilters(
+          Array(EqualTo(partAttr, Literal(7)))).isEmpty)
+        // Both calls filtered one listing, which is what keeps the plan and 
the execution on one
+        // snapshot of the file index.
+        assert(partitionsCalls === 1, s"expected one listing, got 
$partitionsCalls")

Review Comment:
   Done in 2acbef9. The test now calls `planInputPartitions()`, filters on 
`part = 7` (empty), calls `planInputPartitions()` again, then filters on `part 
= 3` (`Seq(3)`), and still expects `partitionsCalls === 1`. Changing 
`planInputPartitions()` back to `partitions.toArray` fails it.



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