feat(transaction): add RowDeltaAction to commit data and delete files in one snapshot#2785
Draft
steveis wants to merge 3 commits into
Draft
feat(transaction): add RowDeltaAction to commit data and delete files in one snapshot#2785steveis wants to merge 3 commits into
steveis wants to merge 3 commits into
Conversation
… in one snapshot Adds the commit-side primitive for merge-on-read row-level changes, equivalent to Table.newRowDelta() in iceberg-java: a transaction action that commits data files and delete files (position or equality deletes) together in a single overwrite snapshot. Both receive the new snapshot's sequence number, so the delete files apply only to strictly older data, which gives upsert semantics when equality deletes are paired with the rows' new values. SnapshotProducer gains added_delete_files and writes a deletes-content manifest alongside the data manifest; the snapshot summary now counts delete files via the existing SnapshotSummaryCollector support. Includes an end-to-end merge-on-read round trip test: parquet data plus an equality-delete upsert committed via row_delta and read back through the scan path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s for RowDelta Review feedback (thanks to Andrei Tserakhau's iceberg-go experience with the same feature): - select the snapshot operation from the delta's content, matching iceberg-java's BaseRowDelta: data-only = append, deletes-only = delete, both = overwrite (was: always overwrite) - write-time guards: reject equality delete files with an empty equality_ids set (would match every row), with unknown field ids, or keyed on float/double columns (equality undefined: NaN, signed zero) - document that BaseRowDelta's conflict-validation surface is deliberately deferred; sequence-number semantics carry row-level correctness (tested: the round-trip test upserts a PK via same-commit equality delete + re-insert and the scan sees the new row)
Author
|
Pushed 6ed0879 addressing review feedback from Andrei Tserakhau (iceberg-go implementer of the same feature — feedback received by email):
Already covered, for other reviewers' reference: the sequence-number rule lives in the reader ( |
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.
Which issue does this PR close?
DeltaWriter) and of Copy-on-Write and Merge-on-Read support #2186 (merge-on-read support).What changes are included in this PR?
Adds
Transaction::row_delta(), the equivalent of iceberg-java'sTable.newRowDelta(): a transaction action that commits data files and delete files (position or equality deletes) together in a singleoverwritesnapshot.RowDeltaActionmirrorsFastAppendActionand addsadd_delete_files().SnapshotProducergainsadded_delete_filesand writes a deletes-content manifest alongside the data manifest (theManifestContentType::Deletesbuilders already existed but had no producer).SnapshotSummaryCollectordelete-file support.Semantics: both the added data files and the added delete files receive the new snapshot's sequence number, so per the spec the delete files apply only to strictly older data. Pairing an equality delete for a key with the key's new row therefore gives upsert semantics in one atomic commit — the standard shape for CDC producers.
Context: this crate's read path already applies delete files; this PR adds the missing write/commit side. We use it in production-shaped testing in a Postgres→Iceberg CDC pipeline (sustained soaks of consecutive row-delta commits, differential-checked against Postgres), with the results read back by this crate and by iceberg-datafusion.
Are these changes tested?
Unit tests mirroring the fast-append suite (validation of content types, empty action, deletes-only commits, manifest contents and sequence numbers), plus an end-to-end merge-on-read round trip (
test_row_delta_write_and_read_back): real parquet data written, an equality-delete upsert committed viarow_delta, and the table scanned back through the crate's own reader, asserting the upserted values.cargo test -p iceberg --libpasses (1389 tests).AI assistance disclosure
Per the Guidelines for AI-assisted Contributions: this PR was drafted with AI assistance (Anthropic's Claude, via Claude Code), then reviewed, tested, and validated by the author.
cargo fmt,clippy, the full unit suite, and the public-api snapshot were run locally; the feature is also exercised continuously in our CDC pipeline against this branch.Areas reviewers may want to focus on (our known uncertainties):
SnapshotSummaryCollector; we believe the added-delete-files accounting matches iceberg-java but would appreciate a check.RowDeltaOperation::existing_manifestreusesFastAppendOperation's retain-all filter (including the FastAppendAction drops delete-only manifests, causing deleted files to reappear #2148 delete-only-manifest fix) rather than partition-pruning like java's overwrite paths; intentional for a pure row delta, but worth a look.