Delta table scan should be optimized when deletion vectors don't exist [databricks] by jihoonson · Pull Request #13980 · NVIDIA/cudf-spark

jihoonson · 2025-12-08T22:01:32Z

Description

Currently, when reading on GPU , the plugin cannot support certain optimizations for Delta tables such as file split or predicate pushdown if the deletion vectors are enabled for them. This change was made in #13843. However, we accidentally disabled the optimizations even for Delta tables without deletion vectors. This PR fixes this regression by disabling the optimization for Delta tables if they have deletion vectors.

Checklists

This PR has added documentation for new or modified features or behaviors.
This PR has added new tests or modified existing tests to cover new code paths.
(Please explain in the PR description how the new code paths are tested, such as names of the new/existing tests that cover them.)
Performance testing has been performed and its results are added in the PR description. Or, an issue has been filed with a link in the PR description.

Signed-off-by: Jihoon Son <ghoonson@gmail.com>

greptile-apps · 2025-12-08T22:06:36Z

Greptile Overview

Greptile Summary

Fixed regression from PR #13843 where file splitting and predicate pushdown optimizations were disabled for all Delta tables. Now optimizations are only disabled when deletion vectors actually exist (fmt.hasTablePath is true).

Key Changes:

Changed hardcoded optimizationsEnabled = false to conditional logic that checks fmt.hasTablePath
When deletion vectors exist (hasTablePath is true), optimizations are disabled with a warning message
When deletion vectors don't exist, fmt.optimizationsEnabled is used, restoring the original behavior
Added comprehensive integration tests covering 5 scenarios including edge cases

Confidence Score: 4/5

This PR is safe to merge with the caveat that the correctness relies on Delta Lake's tablePath field accurately indicating deletion vector presence
The fix correctly addresses the regression by conditionally enabling optimizations. The logic is simple and mirrors the original pre-regression behavior. Comprehensive tests cover multiple edge cases. Score reduced from 5 to 4 because the fix relies on the assumption that fmt.hasTablePath accurately indicates deletion vector presence, which is not explicitly verified in this PR but appears to be a Delta Lake contract based on code comments
No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
delta-lake/delta-33x/src/main/scala/com/nvidia/spark/rapids/delta/delta33x/Delta33xProvider.scala	5/5	Changed from hardcoded `false` to conditional logic based on `hasTablePath`, enabling file splitting and predicate pushdown when deletion vectors don't exist
delta-lake/delta-40x/src/main/scala/com/nvidia/spark/rapids/delta/delta40x/Delta40xProvider.scala	5/5	Same fix as Delta33x - changed from hardcoded `false` to conditional logic based on `hasTablePath`
integration_tests/src/main/python/delta_lake_test.py	5/5	Added comprehensive tests covering 5 scenarios: no DV, DV enabled without DVs, DV enabled with DVs, DV disabled with DVs, and DVs materialized with REORG

Sequence Diagram

sequenceDiagram
    participant DeltaScan as Delta Table Scan
    participant Provider as Delta33x/40x Provider
    participant FileFormat as DeltaParquetFileFormat
    participant GPU as GPU Parquet Reader
    
    DeltaScan->>Provider: toGpuParquetFileFormat(fmt)
    Provider->>Provider: Check fmt.hasTablePath
    
    alt Deletion Vectors Exist (hasTablePath == true)
        Provider->>Provider: Set optimizationsEnabled = false
        Provider->>Provider: Log warning about DV limitations
        Provider->>GPU: Create GpuDeltaParquetFileFormat(optimizationsEnabled=false)
        Note over GPU: File splitting DISABLED<br/>Predicate pushdown DISABLED
    else No Deletion Vectors (hasTablePath == false)
        Provider->>Provider: Use fmt.optimizationsEnabled
        Provider->>GPU: Create GpuDeltaParquetFileFormat(optimizationsEnabled=true)
        Note over GPU: File splitting ENABLED<br/>Predicate pushdown ENABLED
    end
    
    GPU->>DeltaScan: Return configured file format

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

jihoonson · 2025-12-09T19:50:14Z

build

jihoonson · 2025-12-09T21:26:12Z

build

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

…r return value check

jihoonson · 2025-12-09T22:25:38Z

build

greptile-apps

Additional Comments (1)

delta-lake/delta-33x/src/main/scala/com/nvidia/spark/rapids/delta/delta33x/Delta33xProvider.scala, line 76-91 (link)

style: Check that Delta40xProvider also gets this fix. PR #13843 disabled optimizations unconditionally in delta-lake/delta-40x/src/main/scala/com/nvidia/spark/rapids/delta/delta40x/Delta40xProvider.scala:80, causing the same regression there.

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

jihoonson · 2025-12-09T22:48:14Z

build

greptile-apps

Additional Comments (1)

delta-lake/delta-40x/src/main/scala/com/nvidia/spark/rapids/delta/delta40x/Delta40xProvider.scala, line 77 (link)

syntax: Missing import for DeltaConfigs. This will cause a compilation error.

Add this to the imports at the top of the file (line 24, similar to Delta33xProvider.scala:24).

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps

Additional Comments (1)

integration_tests/src/main/python/delta_lake_test.py, line 152 (link)

style: Misleading skip reason. The condition not is_databricks_runtime() skips when NOT on Databricks, but the message says "not supported on Databricks". Consider updating to: "This test only runs on Databricks runtime"

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

jihoonson · 2025-12-10T01:04:40Z

build

greptile-apps

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

sameerz · 2025-12-10T19:50:37Z

@jihoonson please retarget to release/25.12

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

jihoonson · 2025-12-12T01:18:32Z

build

greptile-apps

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

gerashegalov

LGTM, but stil questions about actionability of the warning

jihoonson · 2025-12-12T02:34:29Z

build

greptile-apps

_{4 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

gerashegalov

LGTM

gerashegalov · 2025-12-12T02:57:21Z

build

pxLi · 2025-12-12T03:24:20Z

The previous trigger failed due to a blossom glitch.

  shell: /home/github/bin/bash -e {0}
  env:
    OPERATION: START-CI-JOB
    CI_SERVER: ***
    REPO_TOKEN: ***
Failed to create new client

rekicked

pxLi · 2025-12-12T06:32:31Z

build

…into fix-delta-split

greptile-apps

_{5 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

…into fix-delta-split

jihoonson · 2025-12-13T00:39:43Z

build

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

razajafri · 2025-12-13T02:16:43Z

-  override protected def toGpuParquetFileFormat(fmt: DeltaParquetFileFormat): FileFormat =
+  override protected def toGpuParquetFileFormat(fmt: DeltaParquetFileFormat): FileFormat = {
+    val optimizationsEnabled = if (fmt.hasTablePath) {
+      logWarning("Input Delta table has deletion vectors. Optimizations such as file splitting " +


Why are we printing this warning twice? Once here and again in the file format?

We are not. Perhaps your view is messed up by old commits. Please refresh your browser and check again.

gerashegalov

LGTM

enable split for delta tables without deletion vectors enabled

b615e16

Signed-off-by: Jihoon Son <ghoonson@gmail.com>

greptile-apps Bot reviewed Dec 8, 2025

View reviewed changes

jihoonson requested a review from a team December 8, 2025 22:15

razajafri reviewed Dec 9, 2025

View reviewed changes

Comment thread integration_tests/src/main/python/delta_lake_test.py Outdated

use perfile for databricks

e8b28ff

greptile-apps Bot reviewed Dec 9, 2025

View reviewed changes

return num_partitions in gpu_session and revert unnecessary change fo…

2096bb6

…r return value check

greptile-apps Bot reviewed Dec 9, 2025

View reviewed changes

fix it for delta 40

05f69e2

greptile-apps Bot reviewed Dec 9, 2025

View reviewed changes

missing import

f17f1e1

razajafri previously approved these changes Dec 10, 2025

View reviewed changes

disable test for databricks

83e4d70

jihoonson dismissed razajafri’s stale review via 83e4d70 December 10, 2025 00:52

greptile-apps Bot reviewed Dec 10, 2025

View reviewed changes

fix wrong skipif condition

6dcc155

greptile-apps Bot reviewed Dec 10, 2025

View reviewed changes

gerashegalov reviewed Dec 10, 2025

View reviewed changes

disable test_delta_scan_split on databricks

50fed1f

jihoonson changed the base branch from main to release/25.12 December 10, 2025 22:36

revert unrelated change

f361829

greptile-apps Bot reviewed Dec 10, 2025

View reviewed changes

use a util function to get config value

8283b33

check tablePath instead of invasive change

f6a2b70

jihoonson changed the title ~~Delta tables should be splittable when deletion vectors don't exist [databricks]~~ Delta tables should be splittable when deletion vectors don't exist Dec 12, 2025

greptile-apps Bot reviewed Dec 12, 2025

View reviewed changes

gerashegalov reviewed Dec 12, 2025

View reviewed changes

Comment thread ...delta-33x-40x/scala/com/nvidia/spark/rapids/delta/common/GpuDeltaParquetFileFormatBase.scala Outdated

make the warning actionable

e9d1849

greptile-apps Bot reviewed Dec 12, 2025

View reviewed changes

gerashegalov previously approved these changes Dec 12, 2025

View reviewed changes

Comment thread ...delta-33x-40x/scala/com/nvidia/spark/rapids/delta/common/GpuDeltaParquetFileFormatBase.scala Outdated

pxLi changed the title ~~Delta tables should be splittable when deletion vectors don't exist~~ Delta tables should be splittable when deletion vectors don't exist [databricks] Dec 12, 2025

Merge branch 'release/25.12' of https://github.com/NVIDIA/spark-rapids …

cd6827f

…into fix-delta-split

jihoonson mentioned this pull request Dec 12, 2025

[BUG] test_delta_deletion_vector_read_drop_row_group fails with https://github.com/NVIDIA/spark-rapids/pull/13980 #14004

Closed

xfail test_delta_deletion_vector_read_drop_row_group

a47d89b

jihoonson dismissed gerashegalov’s stale review via a47d89b December 12, 2025 20:40

greptile-apps Bot reviewed Dec 12, 2025

View reviewed changes

jihoonson added 2 commits December 12, 2025 16:38

fix predicate pushdown

3afea8a

Merge branch 'release/25.12' of https://github.com/NVIDIA/spark-rapids …

0580a13

…into fix-delta-split

jihoonson changed the title ~~Delta tables should be splittable when deletion vectors don't exist [databricks]~~ Delta table scan should be optimized when deletion vectors don't exist [databricks] Dec 13, 2025

greptile-apps Bot reviewed Dec 13, 2025

View reviewed changes

gerashegalov approved these changes Dec 13, 2025

View reviewed changes

razajafri reviewed Dec 13, 2025

View reviewed changes

razajafri approved these changes Dec 13, 2025

View reviewed changes

gerashegalov approved these changes Dec 13, 2025

View reviewed changes

jihoonson merged commit 9c95eaa into NVIDIA:release/25.12 Dec 13, 2025
65 checks passed

revans2 mentioned this pull request Dec 15, 2025

[BUG] Delta Lake deletion vector tests failing on Spark != 3.5 due to version incompatibilities #14009

Closed

sameerz added the bug Something isn't working label Dec 19, 2025

Uh oh!

Conversation

jihoonson commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklists

Uh oh!

greptile-apps Bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jihoonson commented Dec 9, 2025

Uh oh!

Uh oh!

jihoonson commented Dec 9, 2025

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jihoonson commented Dec 9, 2025

Uh oh!

greptile-apps Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Additional Comments (1)

Uh oh!

jihoonson commented Dec 9, 2025

Uh oh!

greptile-apps Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Additional Comments (1)

Uh oh!

greptile-apps Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Additional Comments (1)

Uh oh!

jihoonson commented Dec 10, 2025

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sameerz commented Dec 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jihoonson commented Dec 12, 2025

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gerashegalov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jihoonson commented Dec 12, 2025

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gerashegalov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gerashegalov commented Dec 12, 2025

Uh oh!

pxLi commented Dec 12, 2025

Uh oh!

jihoonson commented Dec 8, 2025 •

edited

Loading

greptile-apps Bot commented Dec 8, 2025 •

edited

Loading

greptile-apps Bot left a comment •

edited

Loading

greptile-apps Bot left a comment •

edited

Loading

greptile-apps Bot left a comment •

edited

Loading