dwsmith1983 commented on code in PR #5365:
URL: https://github.com/apache/datafusion-comet/pull/5365#discussion_r4084934139


##########
contrib/delta-spark/src/test/scala/org/apache/comet/contrib/delta/CometDeltaNativeScanSuite.scala:
##########
@@ -0,0 +1,3584 @@
+/*
+ * 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.comet.contrib.delta
+
+import java.io.File
+
+import scala.collection.mutable
+import scala.collection.mutable.ListBuffer
+import scala.concurrent.duration.DurationInt
+
+import org.apache.spark.scheduler.{SparkListener, SparkListenerTaskEnd}
+import org.apache.spark.sql.{DataFrame, Row}
+import org.apache.spark.sql.catalyst.expressions.{AttributeReference, 
DynamicPruningExpression, NamedExpression, StructsToJson}
+import org.apache.spark.sql.comet.CometDeltaNativeScanExec
+import org.apache.spark.sql.execution.{FileSourceScanExec, QueryExecution, 
ScalarSubquery, SparkPlan, SubqueryExec}
+import org.apache.spark.sql.execution.datasources.v2.V2TableWriteExec
+import org.apache.spark.sql.functions.{col, lit, to_json}
+import org.apache.spark.sql.internal.SQLConf
+import org.apache.spark.sql.types.{ByteType, LongType, StringType, 
StructField, StructType}
+import org.apache.spark.sql.util.QueryExecutionListener
+
+import org.apache.comet.CometConf
+import org.apache.comet.CometSparkSessionExtensions.isSpark40Plus
+import org.apache.comet.ExtendedExplainInfo
+import org.apache.comet.serde.OperatorOuterClass
+import org.apache.comet.serde.operator.CometNativeScan
+
+/**
+ * Differential suite: append-only Delta tables read through the native Delta 
scan must produce
+ * results identical to Spark's Delta reader, engage the native operator, and 
prune at row-group
+ * and page level.
+ */
+class CometDeltaNativeScanSuite extends CometDeltaTestBase {
+
+  test("plain delta table reads natively with identical results") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark
+        .range(0, 1000)
+        .selectExpr("id", "id * 2 as v", "cast(id as string) as s")
+        .write
+        .format("delta")
+        .save(path)
+
+      val df = spark.read.format("delta").load(path).filter(col("id") > 500)
+      checkDeltaNativeScanAnswer(df)
+    }
+  }
+
+  test("projection and filter on delta table") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark
+        .range(0, 1000)
+        .selectExpr("id", "id % 10 as bucket", "cast(id as double) as d")
+        .write
+        .format("delta")
+        .save(path)
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .select("bucket", "d")
+        .filter(col("d") < 100.0)
+      checkDeltaNativeScanAnswer(df)
+    }
+  }
+
+  test("partitioned delta table with partition filter") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark
+        .range(0, 1000)
+        .selectExpr("id", "id % 7 as p")
+        .write
+        .format("delta")
+        .partitionBy("p")
+        .save(path)
+
+      val df = spark.read.format("delta").load(path).filter(col("p") === 3)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.count() > 0)
+    }
+  }
+
+  test("multi-file delta table after several appends") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      for (i <- 0 until 4) {
+        spark
+          .range(i * 100, (i + 1) * 100)
+          .selectExpr("id", "id * 3 as v")
+          .write
+          .format("delta")
+          .mode("append")
+          .save(path)
+      }
+      val df = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.count() == 400)
+    }
+  }
+
+  test("time travel VERSION AS OF reads natively") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark.range(0, 100).write.format("delta").save(path)
+      spark.range(100, 200).write.format("delta").mode("append").save(path)
+
+      val v0 = spark.read.format("delta").option("versionAsOf", 0).load(path)
+      checkDeltaNativeScanAnswer(v0)
+      assert(v0.count() == 100)
+    }
+  }
+
+  test("selective predicate prunes row groups and pages") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      // Small row groups + page-level stats: sorted data so min/max stats are 
tight. The Delta
+      // writer ignores parquet.* DataFrameWriter options, so set them on the 
Hadoop conf.
+      val hadoopConf = spark.sparkContext.hadoopConfiguration
+      val oldBlockSize = hadoopConf.get("parquet.block.size")
+      val oldPageSize = hadoopConf.get("parquet.page.size")
+      hadoopConf.setInt("parquet.block.size", 256 * 1024)
+      hadoopConf.setInt("parquet.page.size", 16 * 1024)
+      try {
+        spark
+          .range(0, 500000)
+          .selectExpr("id", "id * 2 as v")
+          .sort("id")
+          .coalesce(1)
+          .write
+          .format("delta")
+          .save(path)
+      } finally {
+        if (oldBlockSize == null) hadoopConf.unset("parquet.block.size")
+        else hadoopConf.set("parquet.block.size", oldBlockSize)
+        if (oldPageSize == null) hadoopConf.unset("parquet.page.size")
+        else hadoopConf.set("parquet.page.size", oldPageSize)
+      }
+
+      def query = spark.read
+        .format("delta")
+        .load(path)
+        .filter(col("id") >= 100 && col("id") < 200)
+      checkDeltaNativeScanAnswer(query)
+
+      // checkSparkAnswer re-plans the query, so read metrics from a DataFrame 
we execute
+      // ourselves (collect() runs THIS Dataset's queryExecution; count() 
would plan a new one):
+      // its executed plan holds the metric objects native execution updated.
+      val df = query
+      assert(df.collect().length == 100)
+      val scans = deltaNativeScans(df)
+      assert(scans.size == 1)
+      val metrics = scans.head.metrics
+      val rowGroupsPruned = 
metrics.get("row_groups_pruned_statistics").map(_.value).getOrElse(0L)
+      val pagesPruned = 
metrics.get("page_index_rows_pruned").map(_.value).getOrElse(0L)
+      assert(
+        rowGroupsPruned > 0,
+        s"expected row-group pruning; metrics: ${metrics.map { case (k, v) => 
s"$k=${v.value}" }}")
+      assert(
+        pagesPruned > 0,
+        s"expected page-index pruning; metrics: ${metrics.map { case (k, v) =>
+            s"$k=${v.value}"
+          }}")
+    }
+  }
+
+  test("scalar subquery data filter is pushed down and prunes row groups and 
pages") {
+    withTempPath { dir =>
+      val path = s"${dir.getAbsolutePath}/data"
+      val thresholds = s"${dir.getAbsolutePath}/thresholds"
+      // Same layout as the selective-predicate test: small row groups + tight 
page stats.
+      val hadoopConf = spark.sparkContext.hadoopConfiguration
+      val oldBlockSize = hadoopConf.get("parquet.block.size")
+      val oldPageSize = hadoopConf.get("parquet.page.size")
+      hadoopConf.setInt("parquet.block.size", 256 * 1024)
+      hadoopConf.setInt("parquet.page.size", 16 * 1024)
+      try {
+        spark
+          .range(0, 500000)
+          .selectExpr("id", "id * 2 as v")
+          .sort("id")
+          .coalesce(1)
+          .write
+          .format("delta")
+          .save(path)
+      } finally {
+        if (oldBlockSize == null) hadoopConf.unset("parquet.block.size")
+        else hadoopConf.set("parquet.block.size", oldBlockSize)
+        if (oldPageSize == null) hadoopConf.unset("parquet.page.size")
+        else hadoopConf.set("parquet.page.size", oldPageSize)
+      }
+      spark
+        .sql("SELECT CAST(100 AS BIGINT) AS lo, CAST(200 AS BIGINT) AS hi")
+        .write
+        .format("delta")
+        .save(thresholds)
+
+      // Scalar subqueries are PlanExpressions: unresolved at planning, so the 
bounds can
+      // only reach the native reader via the execution-time 
resolve-and-append path.
+      def query = spark.sql(
+        s"SELECT * FROM delta.`$path` WHERE id >= (SELECT lo FROM 
delta.`$thresholds`) " +
+          s"AND id < (SELECT hi FROM delta.`$thresholds`)")
+      checkDeltaNativeScanAnswer(query)
+
+      val df = query
+      assert(df.collect().length == 100)
+      // The thresholds table inside the subquery is also claimed natively; 
pick the
+      // main data-table scan by its output.
+      assertSubqueryFilterPushed(df, dataColumn = "v")
+      val scans = deltaNativeScans(df).filter(_.output.exists(_.name == "v"))
+      assert(scans.size == 1)
+      val metrics = scans.head.metrics
+      val rowGroupsPruned = 
metrics.get("row_groups_pruned_statistics").map(_.value).getOrElse(0L)
+      val pagesPruned = 
metrics.get("page_index_rows_pruned").map(_.value).getOrElse(0L)
+      assert(
+        rowGroupsPruned > 0,
+        s"expected row-group pruning from the resolved subquery bounds; 
metrics: ${metrics.map {
+            case (k, v) => s"$k=${v.value}"
+          }}")
+      assert(
+        pagesPruned > 0,
+        s"expected page-index pruning from the resolved subquery bounds; 
metrics: ${metrics.map {
+            case (k, v) => s"$k=${v.value}"
+          }}")
+    }
+  }
+
+  test("deletion vectors: scalar subquery filter composes with DV 
application") {
+    withTempPath { dir =>
+      val path = s"${dir.getAbsolutePath}/data"
+      val thresholds = s"${dir.getAbsolutePath}/thresholds"
+      createDvTable(path, rows = 10000)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+      spark
+        .sql("SELECT CAST(5000 AS BIGINT) AS lo")
+        .write
+        .format("delta")
+        .save(thresholds)
+
+      def query =
+        spark.sql(s"SELECT * FROM delta.`$path` WHERE id >= (SELECT lo FROM 
delta.`$thresholds`)")
+      checkDeltaNativeScanAnswer(query)
+      // Deleted rows must stay deleted with the pushed bound applied in-scan.
+      val df = query
+      val rows = df.collect()
+      assert(rows.length == 2500)
+      assert(rows.forall(r => r.getLong(0) % 2 == 1 && r.getLong(0) >= 5000))
+      assertSubqueryFilterPushed(df, dataColumn = "v")
+    }
+  }
+
+  test("column mapping: scalar subquery filter on a renamed column") {
+    withTempPath { dir =>
+      val path = s"${dir.getAbsolutePath}/data"
+      val thresholds = s"${dir.getAbsolutePath}/thresholds"
+      spark.range(0, 1000).selectExpr("id", "id * 2 as 
v").write.format("delta").save(path)
+      enableColumnMapping(path)
+      spark.sql(s"ALTER TABLE delta.`$path` RENAME COLUMN v TO w")
+      spark
+        .sql("SELECT CAST(900 AS BIGINT) AS lo")
+        .write
+        .format("delta")
+        .save(thresholds)
+
+      // The pushed filter references the renamed column: it must bind against 
the
+      // physical read schema, not the logical name.
+      def query =
+        spark.sql(s"SELECT * FROM delta.`$path` WHERE w >= (SELECT lo FROM 
delta.`$thresholds`)")
+      checkDeltaNativeScanAnswer(query)
+      val df = query
+      assert(df.collect().length == 550)
+      assertSubqueryFilterPushed(df, dataColumn = "w")
+    }
+  }
+
+  /**
+   * Assert the resolved scalar-subquery bound was actually appended to the 
native scan's
+   * execution-time common data (answers alone cannot show this: Spark's 
covering FilterExec would
+   * mask a silently-skipped pushdown). `df` must already have been executed.
+   */
+  private def assertSubqueryFilterPushed(df: DataFrame, dataColumn: String): 
Unit = {
+    val scans = deltaNativeScans(df).collect {
+      case s: CometDeltaNativeScanExec if s.output.exists(_.name == 
dataColumn) => s
+    }
+    assert(scans.size == 1)
+    val scan = scans.head
+    val planTimeFilters =
+      
DeltaSparkScanEnvelope.unpack(scan.nativeOp).getCommon.getDataFiltersCount
+    val executedFilters = OperatorOuterClass.DeltaSparkScan
+      .parseFrom(scan.commonData)
+      .getCommon
+      .getDataFiltersCount
+    assert(
+      executedFilters > planTimeFilters,
+      "expected resolved subquery filters appended at execution: " +
+        s"plan-time=$planTimeFilters executed=$executedFilters " +
+        s"dataFilters=${scan.dataFilters.mkString("; ")}")
+  }
+
+  test("scalar subquery filter is NOT pushed below a limit") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark.range(0, 3).selectExpr("id").write.format("delta").save(path)
+      
spark.read.format("delta").load(path).createOrReplaceTempView("t_limit_pushdown")
+
+      val df = spark.sql(
+        "SELECT id FROM (SELECT id FROM t_limit_pushdown ORDER BY id LIMIT 1) 
q " +
+          "WHERE id > (SELECT max(id) FROM range(1))")
+      checkSparkAnswer(df)
+      assert(df.collect().isEmpty)
+      assertNoSubqueryFilterPushed(df)
+    }
+  }
+
+  test("scalar subquery filter is NOT pushed across a nondeterministic 
projection") {
+    withSQLConf(CometConf.COMET_PARQUET_ROW_FILTER_PUSHDOWN_ENABLED.key -> 
"true") {
+      withTempPath { dir =>
+        val path = dir.getAbsolutePath
+        spark.range(0, 5).coalesce(1).write.format("delta").save(path)
+        
spark.read.format("delta").load(path).createOrReplaceTempView("t_monotonic_id")
+
+        // A deterministic conjunct does not commute with a nondeterministic 
projection: the
+        // subquery bound must not be pushed into the scan below `seq`, or the 
surviving rows'
+        // monotonically_increasing_id() values change and the answer is wrong.
+        val df = spark.sql(
+          "SELECT id FROM (SELECT id, monotonically_increasing_id() AS seq " +
+            "FROM t_monotonic_id) q WHERE id > (SELECT max(id) FROM range(1)) 
AND seq = 1")
+        checkSparkAnswer(df)
+        assert(df.collect().toSeq == Seq(Row(1)))
+        assertNoSubqueryFilterPushed(df)
+      }
+    }
+  }
+
+  /**
+   * Assert no scalar-subquery filter was harvested and pushed into the native 
scan's
+   * execution-time common data: the scan must sit below a non-commuting 
operator (e.g. LIMIT /
+   * TopN), so the covering FilterExec's predicate must stay above it rather 
than move into the
+   * scan. Also confirms the query still engaged the native Delta scan, i.e. 
this exercises the
+   * commutativity guard rather than a plan that fell back to Spark entirely. 
`df` must already
+   * have been executed.
+   */
+  private def assertNoSubqueryFilterPushed(df: DataFrame): Unit = {
+    val scans = deltaNativeScans(df).collect { case s: 
CometDeltaNativeScanExec => s }
+    assert(scans.size == 1, s"expected exactly one native Delta scan; found 
${scans.size}")
+    val scan = scans.head
+    val planTimeFilters =
+      
DeltaSparkScanEnvelope.unpack(scan.nativeOp).getCommon.getDataFiltersCount
+    val executedFilters = OperatorOuterClass.DeltaSparkScan
+      .parseFrom(scan.commonData)
+      .getCommon
+      .getDataFiltersCount
+    assert(
+      executedFilters == planTimeFilters,
+      "expected no subquery filter pushed across the non-commuting operator 
between the " +
+        s"covering filter and the scan: plan-time=$planTimeFilters 
executed=$executedFilters " +
+        s"dataFilters=${scan.dataFilters.mkString("; ")}")
+  }
+
+  test("scalar subquery filter rejected by serde still marks the scan as 
filtered") {
+    withTempPath { dir =>
+      val path = s"${dir.getAbsolutePath}/data"
+      val bounds = s"${dir.getAbsolutePath}/bounds"
+      spark.range(0, 100).selectExpr("id", "id * 2 as 
v").write.format("delta").save(path)
+      spark.sql("SELECT CAST(42 AS BIGINT) AS 
lo").write.format("delta").save(bounds)
+
+      // With EqualNullSafe disabled the resolved bound cannot serialize, yet 
the scan must still
+      // carry has_data_filters so native treats it as a filtered read, 
exactly like core does.
+      withSQLConf("spark.comet.expression.EqualNullSafe.enabled" -> "false") {
+        def query =
+          spark.sql(
+            s"SELECT * FROM delta.`$path` WHERE id <=> (SELECT max(lo) FROM 
delta.`$bounds`)")
+        checkDeltaNativeScanAnswer(query)
+        val df = query
+        assert(df.collect().toSeq == Seq(Row(42L, 84L)))
+        assertUnserializedSubqueryFilterMarksScanFiltered(df, dataColumn = "v")
+      }
+    }
+  }
+
+  test("unserializable scalar subquery filter keeps the safe TIMESTAMP_MILLIS 
conversion") {
+    // Same fixture as core's "filtered TIMESTAMP_MILLIS scans do not convert 
values Spark can
+    // skip": a raw file whose only overflowing millisecond value Spark prunes 
from the footer
+    // statistics once the resolved bound is pushed, so native must not 
convert it either.
+    withTempPath { dir =>
+      val path = s"${dir.getAbsolutePath}/data"
+      val bounds = s"${dir.getAbsolutePath}/bounds"
+      writeRawParquetFile(
+        path,
+        """message root {
+          |  optional int32 id;
+          |  optional int64 ts(TIMESTAMP_MILLIS);
+          |}""".stripMargin) { factory =>
+        (1 to 16).map(id => factory.newGroup().append("id", id).append("ts", 
1717243200000L)) :+
+          factory.newGroup().append("id", 17).append("ts", 9223372036854776L)
+      }
+      spark.sql(s"CONVERT TO DELTA parquet.`$path` NO STATISTICS")
+      spark.sql("SELECT timestamp_seconds(0) AS 
bound").write.format("delta").save(bounds)
+
+      withSQLConf(
+        "spark.comet.expression.EqualNullSafe.enabled" -> "false",
+        "spark.sql.parquet.datetimeRebaseModeInRead" -> "CORRECTED",
+        "spark.sql.parquet.int96RebaseModeInRead" -> "CORRECTED") {
+        def query = spark.sql(
+          s"SELECT id, ts FROM delta.`$path` " +
+            s"WHERE ts <=> (SELECT max(bound) FROM delta.`$bounds`)")
+        // Spark 3.x never pushes subquery filters into its parquet reader and 
converts the
+        // overflowing value itself, so the answer comparison is meaningful on 
Spark 4.0+ only.
+        if (isSpark40Plus) {
+          checkDeltaNativeScanAnswer(query)
+        }
+        val df = query
+        assert(df.collect().isEmpty)
+        assert(
+          deltaNativeScans(df).nonEmpty,
+          s"expected a native Delta scan:\n${df.queryExecution}")
+        assertUnserializedSubqueryFilterMarksScanFiltered(df, dataColumn = 
"ts")
+      }
+    }
+  }
+
+  /**
+   * Assert the execution-time common data of the scan producing `dataColumn` 
reports
+   * `has_data_filters` with no serialized data filter: the plan-time proto 
carries neither, and
+   * the resolved subquery filter is the only data filter, so only the 
execution-time path can set
+   * the bit. `df` must already have been executed.
+   */
+  private def assertUnserializedSubqueryFilterMarksScanFiltered(
+      df: DataFrame,
+      dataColumn: String): Unit = {
+    val scans = deltaNativeScans(df).collect {
+      case s: CometDeltaNativeScanExec if s.output.exists(_.name == 
dataColumn) => s
+    }
+    assert(scans.size == 1, s"expected exactly one native Delta scan; found 
${scans.size}")
+    val scan = scans.head
+    assert(
+      scan.dataFilters.exists(_.exists(_.isInstanceOf[ScalarSubquery])),
+      s"expected a scalar subquery data filter: ${scan.dataFilters.mkString("; 
")}")
+    val planTime = DeltaSparkScanEnvelope.unpack(scan.nativeOp).getCommon
+    assert(!planTime.getHasDataFilters && planTime.getDataFiltersCount == 0)
+    val executed = 
OperatorOuterClass.DeltaSparkScan.parseFrom(scan.commonData).getCommon
+    assert(
+      executed.getHasDataFilters,
+      "expected has_data_filters at execution even though the resolved 
subquery filter did " +
+        s"not serialize: dataFilters=${scan.dataFilters.mkString("; ")}")
+    assert(executed.getDataFiltersCount == 0)
+  }
+
+  test("aggregation over delta table") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark
+        .range(0, 10000)
+        .selectExpr("id", "id % 13 as g", "id * 2 as v")
+        .write
+        .format("delta")
+        .save(path)
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .groupBy("g")
+        .sum("v")
+      checkDeltaNativeScanAnswer(df)
+    }
+  }
+
+  test("conf disables the native delta scan") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark.range(0, 100).write.format("delta").save(path)
+
+      withSQLConf(DeltaScanConf.COMET_DELTA_NATIVE_ENABLED.key -> "false") {
+        val df = spark.read.format("delta").load(path)
+        checkSparkAnswer(df)
+        assert(deltaNativeScans(df).isEmpty)
+      }
+    }
+  }
+
+  test("native delta scan is opt-in: disabled when the conf is not set") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark.range(0, 100).write.format("delta").save(path)
+
+      // The suite base enables the scan globally; drop the key entirely to
+      // observe the out-of-the-box default.
+      spark.conf.unset(DeltaScanConf.COMET_DELTA_NATIVE_ENABLED.key)
+      try {
+        assert(!DeltaScanConf.scanEnabled)
+        val df = spark.read.format("delta").load(path)
+        checkSparkAnswer(df)
+        assert(deltaNativeScans(df).isEmpty)
+      } finally {
+        spark.conf.set(DeltaScanConf.COMET_DELTA_NATIVE_ENABLED.key, "true")
+      }
+    }
+  }
+
+  test("table root under a directory whose name contains a newline falls back 
to Spark") {
+    // object_store recognizes the `file` scheme but rejects the control 
character in the
+    // directory name (`%0A` in the URI), so native execution could not open 
the table where
+    // Spark's Hadoop-backed reader can. The claim gate must decline before 
native planning.
+    withTempPath { dir =>
+      val path = new File(new File(dir, "dir\n"), "data").getAbsolutePath
+      spark.range(0, 100).write.format("delta").save(path)
+
+      val df = spark.read.format("delta").load(path)
+      assert(
+        deltaNativeScans(df).isEmpty,
+        "Expected no native Delta scan under a newline directory:\n" +
+          s"${df.queryExecution.executedPlan}")
+      checkSparkAnswerAndFallbackReason(
+        df,
+        "Native Delta scan cannot open path 'file:" + dir.getAbsolutePath +
+          "/dir%0A/data': object_store rejects it")
+    }
+  }
+
+  test("shallow clone whose source data files sit under a newline directory 
falls back") {
+    // The clone's own root is an ordinary path, so only the selected data 
files (resolved to
+    // the source table's directory) carry the rejected segment: this 
exercises the
+    // selected-paths probe, not the root gate. The reason names the first 
such complete path,
+    // a data file under the source directory.
+    withTempPath { dir =>
+      val sourcePath = new File(new File(dir, "dir\n"), 
"source").getAbsolutePath
+      val clonePath = new File(dir, "clone").getAbsolutePath
+      spark.range(0, 100).write.format("delta").save(sourcePath)
+      spark.sql(s"CREATE TABLE delta.`$clonePath` SHALLOW CLONE 
delta.`$sourcePath`")
+
+      val df = spark.read.format("delta").load(clonePath)
+      assert(
+        deltaNativeScans(df).isEmpty,
+        "Expected no native Delta scan for a clone of a newline-directory 
source:\n" +
+          s"${df.queryExecution.executedPlan}")
+      checkSparkAnswerAndFallbackReason(
+        df,
+        "Native Delta scan cannot open path 'file:" + dir.getAbsolutePath +
+          "/dir%0A/source/")
+    }
+  }
+
+  test("converted Parquet table with a newline in a data file basename falls 
back to Spark") {
+    // CONVERT TO DELTA keeps the existing Parquet file names, so the rejected 
character sits in
+    // the basename rather than a directory segment: the table root and every 
parent directory
+    // pass the path probe, and only a check of the complete selected path can 
decline.
+    withTempPath { dir =>
+      val path = new File(dir, "data").getAbsolutePath
+      spark.range(0, 100).repartition(2).write.parquet(path)
+      val original = new 
File(path).listFiles().filter(_.getName.endsWith(".parquet")).head
+      val renamed = new File(path, "part-00000\n.snappy.parquet")
+      java.nio.file.Files.move(original.toPath, renamed.toPath)
+      spark.sql(s"CONVERT TO DELTA parquet.`$path`")
+
+      val df = spark.read.format("delta").load(path)
+      assert(
+        deltaNativeScans(df).isEmpty,
+        "Expected no native Delta scan for a converted table with a newline 
basename:\n" +
+          s"${df.queryExecution.executedPlan}")
+      checkSparkAnswerAndFallbackReason(
+        df,
+        s"Native Delta scan cannot open path 
'file:$path/part-00000%0A.snappy.parquet': " +
+          "object_store rejects it")
+    }
+  }
+
+  private def createDvTable(path: String, rows: Long = 1000): Unit = {
+    spark.range(0, rows).selectExpr("id", "id * 2 as 
v").write.format("delta").save(path)
+    spark.sql(
+      s"ALTER TABLE delta.`$path` SET TBLPROPERTIES 
('delta.enableDeletionVectors' = 'true')")
+  }
+
+  /**
+   * Same shape as `createDvTable`, plus one extra TINYINT column (value 7) 
under `columnName`.
+   */
+  private def createDvTableWithExtraColumn(
+      path: String,
+      columnName: String,
+      rows: Long = 1000): Unit = {
+    spark
+      .range(0, rows)
+      .selectExpr("id", s"cast(7 as tinyint) as `$columnName`")
+      .write
+      .format("delta")
+      .save(path)
+    spark.sql(
+      s"ALTER TABLE delta.`$path` SET TBLPROPERTIES 
('delta.enableDeletionVectors' = 'true')")
+  }
+
+  test("deletion vectors: DELETE-produced DVs read natively with correct 
results") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      val df = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.count() == 500)
+    }
+  }
+
+  test(
+    "deletion vectors: user column named like the synthetic internal-column 
slot keeps its " +
+      "own values") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      val collidingName = "_comet_delta___delta_internal_is_row_deleted"
+      createDvTableWithExtraColumn(path, collidingName)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id = 0")
+
+      val df = spark.read.format("delta").load(path).select("id", 
collidingName)
+      checkDeltaNativeScanAnswer(df)
+      val survivingValues = 
df.collect().map(_.getAs[Byte](collidingName)).distinct
+      assert(
+        survivingValues.sameElements(Array(7.toByte)),
+        "expected the user column's own value (7) to survive DV filtering, " +
+          s"got ${survivingValues.toSeq}")
+    }
+  }
+
+  test("deletion vectors: normally named extra column alongside DVs reads 
natively") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTableWithExtraColumn(path, "tag")
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id = 0")
+
+      val df = spark.read.format("delta").load(path).select("id", "tag")
+      checkDeltaNativeScanAnswer(df)
+      val survivingValues = df.collect().map(_.getAs[Byte]("tag")).distinct
+      assert(
+        survivingValues.sameElements(Array(7.toByte)),
+        "expected the extra column's value (7) to survive DV filtering, got " +
+          survivingValues.toSeq)
+    }
+  }
+
+  test("deletion vectors: UPDATE-produced DVs read natively with correct 
results") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"UPDATE delta.`$path` SET v = -1 WHERE id < 100")
+
+      val df = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.filter(col("v") === -1).count() == 100)
+      assert(df.count() == 1000)
+    }
+  }
+
+  test("deletion vectors: multiple DELETEs accumulate correctly") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 3 = 0")
+
+      val df = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(df)
+      // odd ids not divisible by 3
+      assert(df.count() == (0L until 1000L).count(i => i % 2 != 0 && i % 3 != 
0))
+    }
+  }
+
+  test(
+    "deletion vectors: maxDeletedRowsPerFile budget declines an oversized DV 
and " +
+      "claims once raised") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      // repartition(4) guarantees >= 2 physical files so the per-file 
cardinality gate has
+      // more than one file to inspect, mirroring design F3's multi-file test 
shape.
+      spark
+        .range(0, 1000)
+        .selectExpr("id", "id * 2 as v")
+        .repartition(4)
+        .write
+        .format("delta")
+        .save(path)
+      spark.sql(
+        s"ALTER TABLE delta.`$path` SET TBLPROPERTIES 
('delta.enableDeletionVectors' = 'true')")
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      withSQLConf(DeltaScanConf.COMET_DELTA_MAX_DELETED_ROWS_PER_FILE.key -> 
"1") {
+        val df = spark.read.format("delta").load(path)
+        checkSparkAnswer(df)
+        assert(
+          deltaNativeScans(df).isEmpty,
+          "a budget of 1 deleted row per file must decline every DV-bearing 
file")
+      }
+
+      withSQLConf(DeltaScanConf.COMET_DELTA_MAX_DELETED_ROWS_PER_FILE.key -> 
"1000000") {
+        val df = spark.read.format("delta").load(path)
+        checkDeltaNativeScanAnswer(df)
+      }
+    }
+  }
+
+  test("deletion vectors: maxDeletedRowsPerFile decline reason names the conf 
key") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      withSQLConf(DeltaScanConf.COMET_DELTA_MAX_DELETED_ROWS_PER_FILE.key -> 
"1") {
+        checkSparkAnswerAndFallbackReason(
+          spark.read.format("delta").load(path),
+          DeltaScanConf.COMET_DELTA_MAX_DELETED_ROWS_PER_FILE.key)
+      }
+    }
+  }
+
+  test("deletion vectors: fully-deleted region and selective predicate still 
prune pages") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      val hadoopConf = spark.sparkContext.hadoopConfiguration
+      val oldBlockSize = hadoopConf.get("parquet.block.size")
+      val oldPageSize = hadoopConf.get("parquet.page.size")
+      hadoopConf.setInt("parquet.block.size", 256 * 1024)
+      hadoopConf.setInt("parquet.page.size", 16 * 1024)
+      try {
+        spark
+          .range(0, 500000)
+          .selectExpr("id", "id * 2 as v")
+          .sort("id")
+          .coalesce(1)
+          .write
+          .format("delta")
+          .save(path)
+      } finally {
+        if (oldBlockSize == null) hadoopConf.unset("parquet.block.size")
+        else hadoopConf.set("parquet.block.size", oldBlockSize)
+        if (oldPageSize == null) hadoopConf.unset("parquet.page.size")
+        else hadoopConf.set("parquet.page.size", oldPageSize)
+      }
+      spark.sql(
+        s"ALTER TABLE delta.`$path` SET TBLPROPERTIES 
('delta.enableDeletionVectors' = 'true')")
+      // Delete a slice inside the predicate range and a large slice outside 
it.
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id >= 150 AND id < 160")
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id >= 300000")
+
+      def query = spark.read
+        .format("delta")
+        .load(path)
+        .filter(col("id") >= 100 && col("id") < 200)
+      checkDeltaNativeScanAnswer(query)
+
+      val df = query
+      assert(df.collect().length == 90)
+      val scans = deltaNativeScans(df)
+      assert(scans.size == 1)
+      val metrics = scans.head.metrics
+      val pagesPruned = 
metrics.get("page_index_rows_pruned").map(_.value).getOrElse(0L)
+      assert(
+        pagesPruned > 0,
+        s"expected page-index pruning to compose with DVs; metrics: 
${metrics.map { case (k, v) =>
+            s"$k=${v.value}"
+          }}")
+    }
+  }
+
+  test("deletion vectors: aggregation over DV table") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path, rows = 10000)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 7 = 0")
+
+      val df = spark.read.format("delta").load(path).groupBy(col("id") % 
13).count()
+      checkDeltaNativeScanAnswer(df)
+    }
+  }
+
+  test("deletion vectors: partitioned table reads natively with correct 
results") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      spark
+        .range(0, 1000)
+        .selectExpr("id", "id % 5 as p", "id * 2 as v")
+        .write
+        .format("delta")
+        .partitionBy("p")
+        .save(path)
+      spark.sql(
+        s"ALTER TABLE delta.`$path` SET TBLPROPERTIES 
('delta.enableDeletionVectors' = 'true')")
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 3 = 0")
+
+      val df = spark.read.format("delta").load(path).filter(col("p") === 2)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.count() == (0L until 1000L).count(i => i % 5 == 2 && i % 3 != 
0))
+
+      val all = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(all)
+      assert(all.count() == (0L until 1000L).count(_ % 3 != 0))
+    }
+  }
+
+  test("deletion vectors: combined with constant metadata columns") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id < 250")
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .selectExpr("id", "v", "_metadata.file_name as fn")
+      checkSparkAnswer(df.selectExpr("id", "v", "length(fn) > 0"))
+      // Whether this claims or declines, results must match; if it claimed, 
verify the
+      // native node is present so the combination is actually exercised when 
supported.
+      val rows = df.collect()
+      assert(rows.length == 750)
+      assert(rows.forall(_.getString(2).nonEmpty))
+    }
+  }
+
+  test(
+    "deletion vectors: constant-metadata field names are deduplicated against 
the physical " +
+      "data and partition schemas") {
+    // End-to-end coverage is not possible here: selecting any `_metadata.*` 
field in the DV
+    // shape always declines today for an unrelated, pre-existing reason -- 
Spark reuses the
+    // scan's own row-index bookkeeping attribute as `_metadata.row_index`'s 
source, and
+    // `DeltaScanSupport.rowIndexUnusedAbove` conservatively treats extracting 
ANY `_metadata`
+    // field as making that attribute live (see "combined with constant 
metadata columns"
+    // above, which hedges its assertions for the same reason). That decline 
fires before
+    // `buildDvScanCommon` ever runs, regardless of collision, so it cannot 
exercise the fix.
+    // Test the builder's dedup logic directly instead, the same way 
`storeUris` and
+    // `mergedObjectStoreOptions` are unit-tested without a live scan.
+    val physicalDataSchema =
+      StructType(Seq(StructField("_comet_metadata_file_path", ByteType)))
+    val physicalPartitionSchema =
+      StructType(Seq(StructField("_comet_metadata_file_size", LongType)))
+    val fileConstantMetadataColumns = Seq(
+      AttributeReference("file_path", StringType, nullable = false)(),
+      AttributeReference("file_size", LongType, nullable = false)())
+
+    val constantMetadataFields = CometNativeScan.uniqueConstantMetadataFields(
+      fileConstantMetadataColumns,
+      physicalDataSchema.fields.map(_.name).toSet ++ 
physicalPartitionSchema.fields
+        .map(_.name)
+        .toSet)
+    assert(
+      constantMetadataFields.map(_.name) == Seq(
+        "_comet_metadata_file_path_",
+        "_comet_metadata_file_size_"),
+      "expected both constant-metadata names to be uniquified on collision, 
got " +
+        s"${constantMetadataFields.map(_.name)}")
+
+    // The DV builder must feed these already-unique names into 
allocateUniqueInternalFields's
+    // reserved set so the internal-column suffix chain stays consistent with 
them.
+    val requiredSchema = StructType(
+      Seq(
+        StructField("id", LongType),
+        StructField(CometDeltaNativeScan.IsRowDeletedColumn, ByteType),
+        StructField(CometDeltaNativeScan.RowIndexColumn, LongType)))
+    val internalFields = CometDeltaNativeScan.allocateUniqueInternalFields(
+      requiredSchema,
+      physicalDataSchema,
+      physicalPartitionSchema,
+      constantMetadataFields)
+
+    val allNames = physicalDataSchema.fields.map(_.name) ++
+      physicalPartitionSchema.fields.map(_.name) ++
+      constantMetadataFields.map(_.name) ++
+      internalFields.map(_.name)
+    assert(allNames.distinct.length == allNames.length, s"expected all names 
distinct: $allNames")
+  }
+
+  test(
+    "non-DV shape: user column named like the synthetic constant-metadata slot 
keeps its " +
+      "own values") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      val collidingName = "_comet_metadata_file_path"
+      spark
+        .range(0, 100)
+        .selectExpr("id", s"cast(7 as tinyint) as `$collidingName`")
+        .write
+        .format("delta")
+        .save(path)
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .selectExpr("id", s"`$collidingName`", "_metadata.file_path as fp")
+      checkDeltaNativeScanAnswer(df)
+      val rows = df.collect()
+      val survivingValues = rows.map(_.getAs[Byte](collidingName)).distinct
+      assert(
+        survivingValues.sameElements(Array(7.toByte)),
+        "expected the user column's own value (7) to survive the 
constant-metadata " +
+          s"collision, got ${survivingValues.toSeq}")
+      assert(
+        rows.forall(_.getString(2).nonEmpty),
+        "expected _metadata.file_path to still report a real path")
+    }
+  }
+
+  test("deletion vectors: special characters in table path") {
+    withTempDir { base =>
+      val dir = new java.io.File(base, "s p a r k %dv% test")
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      val df = spark.read.format("delta").load(path)
+      checkDeltaNativeScanAnswer(df)
+      assert(df.count() == 500)
+    }
+  }
+
+  test("deletion vectors: decline when row_index is consumed via multi-hop 
aliases") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .selectExpr("id", "_metadata.row_index as ri")
+        .selectExpr("id", "ri + 1 as ri2")
+        .filter(col("ri2") > 10)
+      checkSparkAnswer(df)
+      assert(deltaNativeScans(df).isEmpty, "derived row_index consumption must 
decline")
+    }
+  }
+
+  test("deletion vectors: decline when row_index feeds a non-Project 
operator") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .groupBy(col("_metadata.row_index") % 7)
+        .count()
+      checkSparkAnswer(df)
+      assert(deltaNativeScans(df).isEmpty, "aggregate over row_index must 
decline")
+    }
+  }
+
+  test("deletion vectors: decline when _metadata.row_index is referenced above 
the scan") {
+    withTempPath { dir =>
+      val path = dir.getAbsolutePath
+      createDvTable(path)
+      spark.sql(s"DELETE FROM delta.`$path` WHERE id % 2 = 0")
+
+      val df = spark.read
+        .format("delta")
+        .load(path)
+        .selectExpr("id", "_metadata.row_index as ri")
+      checkSparkAnswer(df)
+      assert(
+        deltaNativeScans(df).isEmpty,
+        "plans consuming a real row_index must fall back to Spark")
+    }
+  }
+
+  /**
+   * Every [[SparkPlan]] executed during `body`, captured via a 
[[QueryExecutionListener]] rather
+   * than a returned `DataFrame`'s own plan: a `DataFrameWriter` action such 
as `.write.parquet`
+   * has no result `Dataset` to call `.queryExecution` on, so the write's 
physical plan -- the one
+   * `DeltaScanSupport.declineReason` actually saw -- is only observable this 
way.
+   */
+  private def capturePlansDuring(body: => Unit): Seq[SparkPlan] = {

Review Comment:
   Reproduced with the same 300 ms sleep in `onSuccess`: the write-sink test 
fails with an empty reason list. Both copies of `capturePlansDuring` now call 
`CometListenerBusUtils.waitUntilEmpty(spark.sparkContext)` after `body`, inside 
the `try`, the way `CometIcebergTestBase.capturePlans` does. With the sleep 
still in place the suites pass, and they pass on repeated runs with it removed.
   
   The comment on `collectTaskInputMetrics` was stale. That helper had the same 
race and papered over it with `eventually` and a `minRecords` floor. It now 
drains the bus too, the parameter is gone, and its callers keep their floor 
assertions. No other listener-based helper in the contrib tests.
   



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