[GLUTEN][VL] Use authoritative TahoeFileIndex table root for Delta deletion vectors#12612
Open
iemejia wants to merge 3 commits into
Open
[GLUTEN][VL] Use authoritative TahoeFileIndex table root for Delta deletion vectors#12612iemejia wants to merge 3 commits into
iemejia wants to merge 3 commits into
Conversation
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves native Delta Lake deletion-vector (DV) materialization correctness and planning efficiency by sourcing the Delta table root from Delta’s authoritative TahoeFileIndex.path rather than inferring it from data-file paths.
Changes:
- Read the Delta table root from
relation.locationwhen it is aTahoeFileIndex, and thread that root into DV normalization. - Update
DeltaDeletionVectorScanInfo.normalize/extractacross Delta profiles (2.3 / 2.4 / 3.3 / 4.0) to take an explicittablePath: Path, and remove the heuristic table-path resolution (partition-walk +_delta_logexists probe + manual unescape). - Harden tests/benchmarks to validate the supplied table root is actually used, and add a partitioned-table DV integration assertion.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| gluten-delta/src/main/scala/org/apache/gluten/execution/DeltaScanTransformer.scala | Uses TahoeFileIndex.path as the authoritative table root and passes it into DV normalization only for Tahoe-backed Delta scans. |
| gluten-delta/src-delta40/main/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfo.scala | Switches DV normalization/extraction to require an explicit tablePath and removes heuristic root resolution helpers. |
| gluten-delta/src-delta33/main/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfo.scala | Same explicit-root API and heuristic-removal as delta40 profile. |
| gluten-delta/src-delta24/main/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfo.scala | Updates API signature to accept tablePath (no-op behavior remains for pre-3.3). |
| gluten-delta/src-delta23/main/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfo.scala | Updates API signature to accept tablePath (no-op behavior remains for pre-3.3). |
| gluten-delta/src/test/scala/org/apache/gluten/execution/DeltaSuite.scala | Adds an integration test ensuring partitioned-table DELETE produces UUID-based DVs and validates native scan presence where applicable. |
| backends-velox/src-delta40/test/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfoSuite.scala | Updates DV tests for new extract signature and adds a test proving the supplied table root is used for DV materialization. |
| backends-velox/src-delta33/test/scala/org/apache/gluten/delta/DeltaDeletionVectorScanInfoSuite.scala | Same as delta40 test updates/hardening. |
| backends-velox/src-delta33/test/scala/org/apache/spark/sql/execution/benchmark/DeltaPlanningBenchmark.scala | Updates benchmark to use the explicit table-root API for normalize(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ization DeltaScanTransformer already knows the Delta table root via relation.location (a TahoeFileIndex, which PreparedDeltaFileIndex also extends). Thread that path into DeltaDeletionVectorScanInfo.normalize so it no longer re-derives the root from a file path via _delta_log existence probing -- one FileSystem.exists() (an HTTP HEAD on object stores) per partition. normalize gains an optional tablePath parameter; when absent it falls back to the previous resolveTablePath heuristic, so non-TahoeFileIndex locations (e.g. DeltaParquetFileFormat scans without a Tahoe index) and the public single-file extract entry point are unchanged. Add a test to DeltaDeletionVectorScanInfoSuite (delta33 and delta40) asserting the supplied-path and derived-path branches materialize an identical DV payload.
…blePath fallback Now that DeltaScanTransformer passes the table root from TahoeFileIndex.path, make it the single source of truth for DV materialization and remove the previous file-path-derivation fallback. - DeltaDeletionVectorScanInfo.normalize takes a required tablePath: Path (no Option, no fallback); partitionColumnCount is dropped since only the walk-up used it. - Delete resolveTablePath / isDeltaTablePath / unescapePathName and their per-partition _delta_log FileSystem.exists() probing. - DeltaScanTransformer materializes DVs only when relation.location is a TahoeFileIndex (which also covers PreparedDeltaFileIndex); other locations carry no Delta DV metadata and keep the generic split. - Update the public single-file extract(spark, file, tablePath), the delta23/24 stubs, the benchmark, and the suites accordingly. Net ~160 fewer lines. The Hadoop-conf caching and raw on-disk DV byte reading optimizations are retained.
iemejia
force-pushed
the
gluten-delta-tablepath-from-fileindex
branch
from
July 24, 2026 05:04
c5795ce to
10988b1
Compare
|
Run Gluten Clickhouse CI on x86 |
Member
Author
|
The failing PTAL when you get a chance, thanks! |
Member
Author
|
The Both failing jobs are environment/native flakes and just need a re-run. PTAL, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
(Note:
apache/glutenhas nomasterbranch, so this PR targets the default branchmain.)This is a follow-up to #12390 that makes native Delta deletion-vector (DV) materialization both correct and cheaper by sourcing the Delta table root from the authoritative place instead of inferring it.
Background
Delta DV descriptors reference the on-disk bitmap using a table-root-relative UUID path. To read those bitmaps during planning,
DeltaDeletionVectorScanInfo.normalizeneeds the Delta table root. Previously the root was inferred from each data file path via a heuristicresolveTablePath:_delta_logdirectory,_delta_logat each level,%-encoded path characters along the way.This is fragile (it depends on file layout and partition depth, and can resolve the wrong root for non-trivial paths) and it performs at least one
FileSystem.exists("_delta_log")call pernormalizeinvocation.This change
DeltaScanTransformer.getSplitInfosFromPartitionsnow reads the table root directly fromrelation.locationwhen it is aTahoeFileIndex(which also coversPreparedDeltaFileIndexand the other Tahoe subclasses used for time travel, path-based reads, DML, and CDC).TahoeFileIndex.pathis the authoritative Delta table root.DeltaDeletionVectorScanInfo.normalize(partitionFiles, tablePath)andextract(...)across all Delta profiles (2.3 / 2.4 / 3.3 / 4.0).resolveTablePath,isDeltaTablePath(the_delta_logfilesystem probe), and the manualunescapePathNamehelper are removed.DeltaPlanningBenchmarkis updated to the explicit table-root API.Why this is an improvement
normalizecall is eliminated. On local storage this is a small, consistent win; on remote object stores (S3/ABFS/HDFS) anexists()is a network round-trip per DV split, so the saving is proportionally larger there — consistent with the remote-storage motivation of the parent tracking issue [VL] Optimize Delta Lake Deletion Vector processing on remote storage #12399.Test hardening
The DV unit tests were strengthened so they actually prove the supplied root is used: the synthetic
PartitionedFilenow points at an unrelated directory while the real table root is supplied separately, and the tests require a table-root-relative UUID (storageType == "u") DV. The native partitioned-table integration test now asserts that the DELETE really produced an on-disk DV before validating query results, guarding the removal of the old partition-count walk-up.How was this patch tested?
./dev/format-scala-code.sh(JDK 17).DeltaDeletionVectorScanInfoSuiteon Delta 3.3 and Delta 4.0: 4/4 tests pass on each.git diff --checkclean.DeltaPlanningBenchmark(normalize, 100 DV files x 10k rows, 200 timed iters, alternating fresh JVMs) comparing currentmainvsmain+ this change: median 61.40 µs/file vs 61.91 µs/file (~0.8% faster, ~51 µs saved per 100-file call) on local storage, with every run of the change faster than every baseline run. Larger gains are expected on remote filesystems where the removedexists()probe is a network round-trip.Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenCode github-copilot/gpt-5.6-sol