[VL][Delta] Guard DV DML row-index scans#12215
Open
malinjawi wants to merge 1 commit into
Open
Conversation
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
|
Run Gluten Clickhouse CI on x86 |
malinjawi
force-pushed
the
split/delta-dv-dml-scan-safety
branch
from
f124a30 to
6d70f4b
Compare
|
Run Gluten Clickhouse CI on x86 |
Native DELETE/UPDATE/MERGE deletion-vector support deliberately keeps the target row-index scan on Spark until native row-index execution is proven for the required Delta table shapes. Detect those DML row-index scan shapes and keep the small scan subtree (and its parent filter/project) off the native path, so the offload path stays correctness-first while the write side is built out. - add DeltaDeletionVectorDmlUtils to detect and tag Delta DV DML row-index scans (row-index / file-path / bitmap-aggregator references) - gate OffloadDeltaScan to fall back on DML row-index scans unless native Delta write and native DML row-index scan are both enabled - add keepDmlRowIndexFallbackSubtreeOnSpark post-transform rule to keep the filter/project above a fallen-back DML scan on Spark, avoiding row<->columnar transitions right before Delta's JVM bitmap path - cover the fallback shape with DeltaDeletionVectorHandoffSuite cases for Spark 3.5 / Delta 3.3 and Spark 4.0 / Delta 4.0
malinjawi
force-pushed
the
split/delta-dv-dml-scan-safety
branch
from
6d70f4b to
d83aac4
Compare
|
Run Gluten Clickhouse CI on x86 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a safety guard for Delta Lake DML (DELETE/UPDATE/MERGE) on tables with deletion vectors by detecting the special DV “row-index” target scan shape and preventing that scan (and its immediate filter/project) from being offloaded to Gluten/Velox unless explicitly enabled via native flags. This is intended to prevent incorrect DV DML results until native row-index scan support is complete.
Changes:
- Tag Delta DV DML row-index target scans during planning, and use those tags to drive offload/fallback decisions.
- Force Spark fallback for the tagged DV DML row-index scans unless both native-write and native-DML-row-index flags are enabled; keep the parent filter/project on Spark to avoid extra row↔columnar transitions.
- Add Delta 3.3 (Spark 3.5) and Delta 4.0 (Spark 4.0) unit coverage to assert fallback plan shape and repeated-DELETE DV correctness.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| gluten-delta/src/main/scala/org/apache/gluten/extension/OffloadDeltaScan.scala | Adds DV DML row-index scan fallback gating and propagates the DML-scan tag to the transformer when offloaded. |
| gluten-delta/src/main/scala/org/apache/gluten/extension/DeltaPostTransformRules.scala | Adds a post-transform rule to keep the filter/project above a DV DML fallback scan on Spark; expands DV row-index column name handling. |
| gluten-delta/src/main/scala/org/apache/gluten/extension/DeltaDeletionVectorDmlUtils.scala | New utility to detect/tag DV DML row-index scan shapes and provide shared Delta-scan detection helpers. |
| backends-velox/src-delta40/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala | Adds Spark 4 / Delta 4.0 tests asserting DV DML row-index scan fallback and repeated-DELETE DV correctness. |
| backends-velox/src-delta33/test/scala/org/apache/spark/sql/delta/DeltaDeletionVectorHandoffSuite.scala | Adds Spark 3.5 / Delta 3.3 tests asserting DV DML row-index scan fallback and repeated-DELETE DV correctness. |
| backends-velox/src-delta/main/scala/org/apache/gluten/component/VeloxDeltaComponent.scala | Injects the pre-transform tagging rule for DV DML row-index scans in the Velox Delta component. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+199
to
+207
| private def containsDmlRowIndexFallbackScan(plan: SparkPlan): Boolean = { | ||
| plan.exists { | ||
| case scan: FileSourceScanExec => | ||
| FallbackTags | ||
| .getOption(scan) | ||
| .exists(_.reason().contains("fallback Delta DV DML row-index scan")) | ||
| case _ => false | ||
| } | ||
| } |
Comment on lines
+121
to
+122
| val scanColumnNames = (scan.output.map(_.name) ++ scan.requiredSchema.fieldNames).toSet | ||
| scanColumnNames.exists(columnNames.contains) || columnNames.exists(scan.treeString.contains) |
Comment on lines
+125
to
+129
| private def referencesRowIndexColumn(expr: Expression): Boolean = { | ||
| val expressionText = expr.toString() | ||
| expr.references.exists(attr => deletionVectorRowIndexColumnNames.contains(attr.name)) || | ||
| deletionVectorRowIndexColumnNames.exists(expressionText.contains) | ||
| } |
Comment on lines
+131
to
+135
| private def referencesFilePathColumn(expr: Expression): Boolean = { | ||
| val expressionText = expr.toString() | ||
| expr.references.exists(attr => filePathColumnNames.contains(attr.name)) || | ||
| filePathColumnNames.exists(expressionText.contains) | ||
| } |
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?
When you DELETE/UPDATE a Delta table that has deletion vectors, Delta runs a special "row-index" scan on the target to find which rows to mark deleted. Gluten's native scan path can't run that scan correctly yet, so if it gets offloaded the DML can produce wrong results.
This keeps that specific scan (and the filter/project directly above it) on Spark, so DELETE/UPDATE stay correct today. It's a safety guard only — native execution of the row-index scan comes later, gated behind
spark.gluten.sql.delta.enableNativeDmlRowIndexScan+enableNativeWrite(both defaultfalse).Follows the JVM Delta DV scan handoff merged in #12269; rebased onto it.
Main pieces:
DeltaDeletionVectorDmlUtils— detects and tags the DML row-index scan shapeOffloadDeltaScan— falls back on those scans unless both native flags are onkeepDmlRowIndexFallbackSubtreeOnSpark— keeps the parent filter/project on Spark too, so we don't bounce row → columnar → row right before Delta's JVM bitmap pathThe benchmark harness lives in #12217.
Note: uses a string literal for the row-index temp column (not
ParquetFileFormat.ROW_INDEX_TEMPORARY_COLUMN_NAME) so Spark 3.3 / 3.4 compile.Issue: #11901
How was this patch tested?
DeltaDeletionVectorHandoffSuitecovers the fallback plan shape and a repeated-DELETE-over-an-existing-DV correctness case, for Delta 3.3 / Spark 3.5 and Delta 4.0 / Spark 4.0.After rebasing onto
main, re-verified with JDK 17:mvn test-compile -pl backends-velox -am -Pbackends-velox,spark-3.5,delta,spotless:check,git diff --check. CI covers the native runtime lane.Was this patch authored or co-authored using generative AI tooling?
Generated-by: IBM BOB