[RLC] Case 1 — conflict-time data skipping (disjoint data ranges) - #8
Open
sezruby wants to merge 4 commits into
Open
[RLC] Case 1 — conflict-time data skipping (disjoint data ranges)#8sezruby wants to merge 4 commits into
sezruby wants to merge 4 commits into
Conversation
… conflicts Conflict detection filters candidate files by partition only; for unpartitioned (incl. liquid-clustered) tables the append check conflicts on any concurrently-added file. This adds column-statistics data skipping: added files whose stats prove they cannot match the current transaction's read predicates are excluded, so operations touching disjoint data ranges no longer falsely conflict. - DataSkippingReaderBase.buildDataSkippingPredicate + filterFilesByDataSkipping reuse the reader's DataFiltersBuilder / verifyStatsForFilter and the shared parseAndDecodeStats, applying 'expr || !verifyStatsForFilter(...)' for one-way safety (skip only when stats prove no match; missing stats keep the file). - ConflictChecker.getFirstFileMatchingPartitionPredicates applies skipping per read predicate and unions survivors (OR across reads; a read's own data predicates are ANDed), and never skips for a whole-table read. Emits a delta.conflictDetection.dataSkipping.filesSkipped event. - Gated by spark.databricks.delta.conflictDetection.dataSkipping.enabled (internal, default off). - Adds ConflictDataSkippingSuite (disjoint / overlapping / disabled / missing-stats / partitioned). Part of #3. Implements #4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sezruby
force-pushed
the
conflict-data-skipping
branch
from
July 8, 2026 20:38
cacf70d to
a2ad862
Compare
6 tasks
This was referenced Jul 30, 2026
sezruby
added a commit
that referenced
this pull request
Jul 30, 2026
Remove the unsound "Layer 2" append-suppression for rewrite-only DML winners. Keying the append-check skip on the winner's op type (DELETE/UPDATE) is unsound: an UPDATE can move a row into the loser's predicate (winner `SET x=15`, loser `DELETE WHERE x>10`, row was x=5), a genuine write-skew that suppressing the winner's image file would hide. `canSkipAddedFileForRowLevelConcurrency` now skips only the same-file DV union paths (`rowLevelResolvedPaths`). A rewrite-only DML winner's new image files flow into the standard added-files check (conservative abort, one-way safe); reconciling the provably-disjoint case via conflict-time data skipping is owned by Case 1 (#4/#8). Tests reframed to the Layer-1 envelope (9 tests, all passing): the two "both commit" UPDATE cases move to Case 1; DELETE-vs-UPDATE now asserts the image file conservatively conflicts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix a Serializable soundness gap in reader-side conflict-time data skipping. A non-blind-append transaction can record no read predicates (e.g. it removes files without reading the table). With skipping enabled its empty read set produced an empty survivor set, so getFirstFileMatchingPartitionPredicates returned None and a real ConcurrentAppendException was silently suppressed. Skipping is now applied only when the transaction has at least one read predicate; otherwise every concurrently added file stays a conflict candidate, matching behavior with the feature disabled. Add ConflictDataSkippingSuite coverage: - empty read predicates on a non-blind-append txn still conflict (regression test for the fix above); - a whole-table read is never skipped and conflicts with a concurrent append; - filterFilesByDataSkipping AND-combines predicates within a single read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Combine per-read data-skipping into one Spark job and fail safe on evaluation errors, per review comments on delta-io#7358. - filterFilesMatchingAnyReadPredicate(files, dataFiltersPerRead): builds one skipping predicate per read and OR-combines them into a single `where`, so all read predicates are evaluated in one Spark job instead of one job per read. A read with no usable predicate matches everything -> short-circuit and keep all files, no job. - filterFilesByDataSkipping is now the single-read case delegating to it. - Wrap build+eval in try/catch(NonFatal): on failure log a warning and return all candidate files (default conflict behavior), so a skipping error can never abort a valid commit. - ConflictChecker calls the new method once instead of flatMap-per-read. - Test: OR-combines independent reads in one call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move buildDataSkippingPredicate / filterFilesByDataSkipping / filterFilesMatchingAnyReadPredicate out of DataSkippingReaderBase into a new self-typed trait ConflictDataSkippingReader (self: DataSkippingReaderBase), mixed into the base. Existing data-skipping code is left untouched: withStatsInternal0 is reverted to its original inline form, so the only change to DataSkippingReader.scala is the one-line trait mixin. parseAndDecodeStats is duplicated (trait-private) rather than shared with withStatsInternal0, keeping the feature a fully isolated additive unit per review feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Part of the row-level-concurrency umbrella (#3) — Case 1. Full design and rationale: #4.
Summary
Reader-side conflict-time data skipping. Conflict detection currently filters candidate files by partition only. On an unpartitioned table — which includes every liquid-clustered table —
getFirstFileMatchingPartitionPredicatesreturnsfiles.headOption, so the append check conflicts on any concurrently-added file, even when the two operations touch entirely disjoint data ranges. This is the dominant source of false conflicts for liquid clustering, where incremental clustering and DML usually touch different clustering-key regions.This teaches conflict detection to apply the reader's column-statistics data skipping: an added file whose stats prove it cannot match the current transaction's data predicates is excluded from the conflict check.
What it does
DataSkippingReaderBase.filterFilesByDataSkipping(files, dataFilters)— reuses the reader'sDataFiltersBuilder+verifyStatsForFilterto build the skipping predicate, parses each file's stats against the snapshot'sstatsSchema, and appliesexpr || !verifyStatsForFilter(...).ConflictChecker.getFirstFileMatchingPartitionPredicatesconsumes the already-capturedDeltaTableReadPredicate.dataPredicates(previously ignored). Skipping is applied per read predicate with the survivors unioned (OR across independent reads; a read's own predicates are ANDed), and is not applied for a whole-table read — so it can only ever narrow, never miss, a conflict. Emits adelta.conflictDetection.dataSkipping.filesSkippedevent.spark.databricks.delta.conflictDetection.dataSkipping.enabled(internal, default off). No protocol / reader / action changes.Which operations benefit (not just append)
The improved check is the added-files (append) check, which fires whenever the winner adds data files — so it is not only INSERT.
UPDATE/MERGEadd new row-image files, and because those are non-blind changed-data files they are checked under the default WriteSerializable (a blind INSERT is only checked under Serializable). So the headline beneficiaries are concurrent UPDATE / DELETE / MERGE on disjoint clustering ranges (different files), under the default isolation.This complements the same-file DV merge (#5), splitting the space by which check fires:
Roles: the loser supplies the read predicate (
WHERE/ MERGEON); the winner supplies the added files whose stats are pruned. (A DV-DELETE winner re-adds the same file, so that's #5's same-file case, not this PR's.)One-way safety
Data skipping here is sound in one direction only — a file is skipped only when its stats prove no overlap; a wrongly-skipped file would be a missed conflict. Guaranteed by reusing the reader's machinery:
expr || !verifyStatsForFilter(referencedStats)keeps any file whose referenced stats are missing/NULL.tightBounds=false) stats are safe:updateStatsToWideBoundskeeps min/max as valid outer bounds (a superset of live values), so wide bounds only cause fewer skips, never an unsafe one — the same property the reader uses to skip DV'd files. The effect on DV-heavy files is reduced benefit (conservative keep), not incorrectness.filesForScan).Correct across clusters/JVMs by construction — conflict detection is log-based: the loser reads the winner's committed stats from
_delta_logand evaluates skipping against its own local predicates (no shared state; deterministic; other-engine/no-stats writers are kept).Supersedes #5's op-type append-suppression
The #2 POC (issue #5) shortcut the concurrent-
UPDATEcase with a blanket append-suppression for rewrite-only DML winners — skipping all their new image files in the append check, keyed only on the winner's op type. That is unsound: anUPDATEcan flip a row into the loser's predicate (winnerSET x=15, loserDELETE WHERE x>10, row wasx=5), a genuine write-skew the suppression hides. Those image files are ordinary non-blind changed-data files and are correctly arbitrated by this PR's stats-based skip — abort on stats overlap (real flip), reconcile only on proven no-overlap, keep on missing stats. #5 drops the suppression and depends on this PR; they stack (this data skipping under the DV merge).Testing
ConflictDataSkippingSuite(new), phase-locking harness on a Serializable table (10 files,idin [0,1000)):DELETE id<50vs append[1000,1100)) → added file skipped, both commit.DELETE id>=950vs append[1000,1100)) → still conflicts (ConcurrentAppendException).dataSkippingNumIndexedCols=0) → disjoint ranges still conflict (one-way-safety invariant).With the flag off,
getFirstFileMatchingPartitionPredicatesis equivalent to before (candidateFiles = files), so no default-path regression.🤖 Generated with Claude Code