[GLUTEN-12743][VL] Fall back Delta scans requiring generated DV metadata

[felipepessoto]

What changes are proposed in this pull request?

Partially addresses #12743 by fixing the generated deletion-vector metadata scan failures described below. This PR does not resolve the full issue.

Fall back FileSourceScanExec to Delta's JVM reader when spark.databricks.delta.deletionVectors.useMetadataRowIndex=false and the scan requests JVM-generated Delta deletion-vector metadata:

• __delta_internal_row_index
• __delta_internal_is_row_deleted

The fallback checks both scan output and required schema. It no longer depends on the table containing a deletion vector, so DV-free scans requesting these synthetic fields receive the JVM-generated values instead of native NULL/incorrect values. Spark's _metadata.row_index (backed by _tmp_metadata_row_index) and ordinary Delta scans remain eligible for native offload. The existing metadata-row-index-enabled and DV/DML behavior remains scoped as before.

Adds focused Delta 3.3 and Delta 4.0 coverage for non-null generated row indexes, zero deleted-row flags on DV-free files, and continued native offload for ordinary scans and real SQL metadata-row-index reads. Removes the nine corresponding DeltaParquetFileFormatSuite entries from the scheduled known-failures baseline after CI confirmed they now pass.

This PR is independent of #13036 and does not change its row-group fixtures.

How was this patch tested?

• Spark 3.5/Delta 3.3 test compilation and four focused Handoff regressions passed; runtime tests used the current JVM changes on WSL with native CI libraries from [GLUTEN-6887][VL] Daily Update Velox Version (2026_10_14) #13277.
• Scoped Scala formatting/Spotless (including the Spark 4 profile), license-header checks, and git diff --check passed.
• Rebased onto main at b11eaa78e; fresh Delta CI and the Spark matrix are re-running. The previous Delta run passed all eight shards.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot GPT-5.6 Sol

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🟡 Changes recommended

Restrict the fallback to Delta scans and add coverage for _tmp_metadata_row_index.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR adds JVM fallback handling for Delta scans requesting generated deletion-vector metadata when metadata row indexes are disabled, with Delta 3.3 and 4.0 test coverage.

Changes:

• Detects generated metadata in scan output and required schema.
• Preserves native offload for ordinary Delta scans.
• Adds DV-free correctness and offload tests.

File summaries

File	Summary
gluten-delta/src/main/scala/org/apache/gluten/extension/OffloadDeltaScan.scala	Adds fallback logic. Moderate finding (3 votes): restrict fallback to Delta scans. Nit (1 vote): add _tmp_metadata_row_index coverage.
backends-velox/src-delta40/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala	Adds Delta 4.0 fallback, generated metadata, and native-offload tests.
backends-velox/src-delta33/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala	Adds Delta 3.3 fallback, generated metadata, and native-offload tests.

Review details

Suppressed comments (1)

gluten-delta/src/main/scala/org/apache/gluten/extension/OffloadDeltaScan.scala:42

• The new set explicitly adds _tmp_metadata_row_index, but the focused tests only exercise __delta_internal_row_index and __delta_internal_is_row_deleted. The temporary top-level field is a separate generated-column path used by Spark metadata row-index handling, so add coverage for requesting it with useMetadataRowIndex=false and verify the JVM-generated values and fallback; otherwise this newly introduced branch can regress without detection.

  private val generatedDeletionVectorMetadataColumnNames =
    generatedRowIndexColumnNames + DeltaParquetFileFormat.IS_ROW_DELETED_COLUMN_NAME

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🔵 Needs a closer look

Focused tests were blocked by an unavailable native artifact, so final human review is required.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🔵 Needs a closer look

Moderate test assertions must verify ordered row-index/value associations instead of unordered sets.

Review details

Suppressed comments (4)

backends-velox/src-delta33/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala:167

• This assertion converts the generated indexes to a set, so it passes for any permutation (for example, 2, 1, 0) and does not verify that each row retains its file position. Compare the ordered values (or pair the value column with the index) so a misassociated row index cannot pass this regression test.

              .toSet === Set(0L, 1L, 2L))

backends-velox/src-delta33/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala:212

• This assertion converts the generated indexes to a set, so it passes for any permutation (for example, 2, 1, 0) and does not verify that each row retains its file position. Compare the ordered values (or pair the value column with the index) so a misassociated row index cannot pass this regression test.

              .toSet === Set(0L, 1L, 2L))

backends-velox/src-delta40/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala:187

• This assertion converts the generated indexes to a set, so it passes for any permutation (for example, 2, 1, 0) and does not verify that each row retains its file position. Compare the ordered values (or pair the value column with the index) so a misassociated row index cannot pass this regression test.

              .toSet === Set(0L, 1L, 2L))

backends-velox/src-delta40/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala:230

• This assertion converts the generated indexes to a set, so it passes for any permutation (for example, 2, 1, 0) and does not verify that each row retains its file position. Compare the ordered values (or pair the value column with the index) so a misassociated row index cannot pass this regression test.

              .toSet === Set(0L, 1L, 2L))

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🔵 Needs a closer look

Distinguish generated metadata from user columns before applying the fallback.

Review details

Suppressed comments (1)

gluten-delta/src/main/scala/org/apache/gluten/extension/OffloadDeltaScan.scala:105

• scanReadsColumn matches only the field name, so this new predicate also classifies a real Delta data column named __delta_internal_row_index or __delta_internal_is_row_deleted as JVM-generated metadata. Spark's scan shim explicitly notes that metadata/user-column name collisions are possible (shims/spark40/src/main/scala/org/apache/gluten/sql/shims/spark40/Spark40Shims.scala:62-68), and the Delta format allows unrestricted column names. With this setting disabled, selecting such an ordinary Delta column now unnecessarily forces the scan onto Spark, so distinguish Delta-injected metadata from user fields before applying this fallback.

      (name, _) => generatedDeletionVectorMetadataColumnNames.contains(name))

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[felipepessoto]

Replying to this review:

The name-based check matches Delta's reader semantics here. In both Delta 3.3.2 and Delta 4.0.1, buildReaderWithPartitionValues identifies __delta_internal_row_index and __delta_internal_is_row_deleted by exact field name. Their definitions are plain StructFields without a generated-column marker. GlutenDeltaParquetFileFormat follows the same logic.

When useMetadataRowIndex=false, the JVM reader therefore also interprets matching required fields as synthetic metadata, rather than distinguishing physical user columns. A metadata-marker requirement in the fallback would miss legitimate synthetic fields. The rule is already restricted to Delta scans, so unrelated Parquet columns are unaffected.

I'm keeping the fallback unchanged to preserve Delta's JVM reader behavior. This does not rely on assuming that table creation prohibits these names; the internal-column list alone does not establish that.

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved review comments remain, and all supplied assessments indicate approval readiness.

Review effort: Lite
Findings: None

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 1

Open (3)

• This predicate matches only on column name and disregards DataType, which can cause unnecessary… · New
• scanReadsGeneratedDeletionVectorMetadataColumn ignores Spark’s case-sensitivity rules by using… · New
• This test assumes the deleted-row flag column is ByteType (via getByte(0)) but doesn’t assert… · New

[github-actions]

Run Gluten Clickhouse CI on x86

[felipepessoto]

@philo-he, @zhouyuan, @zhztheplayer could you take a look, please?

[zhouyuan]

@felipepessoto thanks for the patch.
@malinjawi Could we support DV read in this case like (#12836)?

[malinjawi]

Thanks @felipepessoto, this looks right to me. Falling back when the scan actually reads the generated columns matches what Delta's reader does with useMetadataRowIndex=false. Left a few comments.

Could you rebase the latest main to make sure CI is still green?

cc: @zhouyuan @zhztheplayer @philo-he

[malinjawi]

@felipepessoto thanks for the patch. @malinjawi Could we support DV read in this case like (#12836)?

@zhouyuan
Not with #12836 alone that PR only moved DV loading to the executors, and native reads still work by dropping deleted rows. The scans in this PR need the actual row index and deleted-row flag as values, which Velox can't produce yet.

We would need to add that natively so I'd land this one first as the correctness fix and build on it.

[github-actions]

Run Gluten Clickhouse CI on x86

[felipepessoto]

@malinjawi Addressed the inline comments in 76ac03f and rebased onto main at b11eaa78e. The fallback now covers only Delta's two internal fields; both versioned suites exercise real _metadata.row_index SQL and use a single deltaLog.update() snapshot.

Four focused Spark 3.5/Delta 3.3 regressions passed locally, including native metadata-row-index reads and the synthetic-column fallbacks. Formatting and header checks passed too. Fresh Delta CI and the Spark matrix are re-running for the rebased head; their results are still pending.

[Copilot]

🟡 Changes recommended

The stated fallback for Spark’s temporary top-level row-index field is not implemented or tested.

1 open finding

• Handle Spark temporary row-index field in metadata fallback · New

3 resolved since last review

• scanReadsGeneratedDeletionVectorMetadataColumn ignores Spark’s case-sensitivity rules by using…
• This predicate matches only on column name and disregards DataType, which can cause unnecessary…
• This test assumes the deleted-row flag column is ByteType (via getByte(0)) but doesn’t assert…

🧠 Review effort: Lite

[felipepessoto]

@malinjawi @zhouyuan I created an exploratory draft PR, it requires considerable changes and doesn't seem worth for something legacy, disabled by default: #13123