feat(transaction): detect concurrent deletes for data files a rewrite removes by vovacf201 · Pull Request #202 · risingwavelabs/iceberg-rust

vovacf201 · 2026-07-29T15:20:08Z

The bug

An operation that rewrites data files (compaction, most importantly) does this:

read snapshot S
apply the deletes visible at S, materializing them into new data files
commit: add the new data files, remove the originals

If a concurrent writer commits a deletion vector for one of the data files being removed between (1)
and (3), that DV's deletes are never materialized — and because removing the data file also
retires the DV as a dangling delete, the rows it deleted silently come back.

Nothing catches this today. validate_data_file_changes checks for duplicate adds and that the
files being deleted exist; the data file does still exist, so no conflict is raised.

This is the analogue of Java's validateNoNewDeletesForDataFiles, which has no equivalent here.

The fix

SnapshotProducer::validate_no_new_deletes_for_data_files(starting_snapshot_id), exposed on
ReplaceFilesAction as:

Transaction::new(&table)
    .rewrite_files()
    .validate_from_snapshot_id(snapshot_id_that_was_read)

It fails the commit if a delete file targeting one of the removed data files was added after that
snapshot, so the caller can retry against the current snapshot where the new deletes are visible.

Opt-in — existing callers are unaffected until they pass a starting snapshot.

Implementation notes

Candidate manifests are narrowed by manifest sequence number (a manifest added at or before the
starting sequence number cannot contain newer entries), then entries are filtered on their own
sequence number, because a newer manifest can carry older entries forward as Existing.
Only delete files that record referenced_data_file are covered — deletion vectors, and position
deletes written for a single data file. That is the only case where the data-file association is in
the manifest and can be checked without reading file contents. Position delete files spanning
several data files, and equality deletes (retired by sequence number rather than by data file), are
documented as out of scope. This matches the granularity is_dangling_delete already uses.
An unknown starting_snapshot_id is an error rather than a silent pass, since it means the
caller's assumption about the table is wrong.

Tests

Four cases, over a fixture that extends the existing make_v2_table_with_delete_manifest with a
newer snapshot whose delete manifest holds a DV — i.e. exactly the state a concurrent writer
leaves behind:

test_validate_rejects_new_deletion_vector_for_a_rewritten_data_file — the conflict; asserts the
error names both the data file and the new delete file
test_validate_allows_new_deletion_vector_for_an_untouched_data_file — a DV for an unrelated data
file must not conflict (guards against over-rejecting, which would break compaction)
test_validate_allows_when_nothing_was_committed_concurrently
test_validate_rejects_unknown_starting_snapshot

Confirmed non-vacuous by stubbing the validation to return Ok(()), which makes both rejection tests
fail.

Verification

make check-fmt, make check-clippy (--all-targets --all-features --workspace -- -D warnings,
exit 0), cargo test -p iceberg --lib (1292 passed, 0 failed) on the pinned nightly-2025-10-27.

Follow-up

The corresponding change in iceberg-compaction is to pass the snapshot it planned from — its
compaction/mod.rs already carries an explicit // TODO: support validation of data files and delete files with starting snapshot. I'll open that once this API settles, since it depends on the shape
here.

Independent of #200 and #201 (different files).

…eberg-rust#202 Not for merge as-is. This pins the `iceberg*` deps to `b087b307`, the head of risingwavelabs/iceberg-rust#202, which adds the `validate_from_snapshot_id` API the previous commit calls. Cargo resolves it because GitHub keeps a PR head reachable in the upstream repository, so CI can build and test the change while that PR is in review. Re-pin to the merge commit on `dev_rebase_main_20260303` once #202 lands, then this commit can be dropped or amended. All five `iceberg*` deps are moved together on purpose: pinning only one would pull in two incompatible copies of the `iceberg` crate.

chenzl25 · 2026-07-31T05:48:20Z

@Li0k We need to take a close look at this PR because it deals with conflict detection between concurrent writes and compaction commits. This is especially important for COW mode, where different branches are involved.

chenzl25

Requesting changes because this validation can still succeed while concurrent deletes are lost:

Position deletes with referenced_data_file=None are skipped, even when bounds identify the target file.
Equality deletes are only safe when replacement files use the starting snapshot’s sequence number, but the API does not enforce that prerequisite.
starting_snapshot_id is not verified to be an ancestor of the target branch.

Please enforce these conditions and add negative tests before exposing this as a correctness guarantee.

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

…quence number Follow-up to the review of risingwavelabs/iceberg-rust#202, which hardened the validation: it now requires the replacement files to carry the starting snapshot's sequence number, because that is what keeps pre-existing equality deletes applicable to them. The `use_starting_sequence_number == false` path deliberately does not preserve it, so asking for validation there would just fail the commit. Drop the call on that branch and say why, rather than leave a guarantee that cannot hold. Concurrent-delete protection therefore requires `use_starting_sequence_number`. Also re-pin to the current head of #202 (84b9670b) so CI exercises the hardened validation.

vovacf201 · 2026-07-31T09:33:44Z

Requesting changes because this validation can still succeed while concurrent deletes are lost:

Position deletes with referenced_data_file=None are skipped, even when bounds identify the target file.

Equality deletes are only safe when replacement files use the starting snapshot’s sequence number, but the API does not enforce that prerequisite.

starting_snapshot_id is not verified to be an ancestor of the target branch.

Please enforce these conditions and add negative tests before exposing this as a correctness guarantee.

Six tests added, including the two must-not-over-reject directions (a bounds-identified delete for an untouched file; a new equality delete once the prerequisite holds). Each fix is mutation-verified.

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

Supersedes the temporary pin at risingwavelabs/iceberg-rust#202's head. That only carried the concurrent-delete validation this change depends on; the fork branch carries #200, #201 and #202 together, so a consumer pinning here gets all of them in one hop instead of needing three separate pins. Pinned by revision rather than branch name so the build stays reproducible even as that branch moves. Not for upstream. This is a deliberately divergent pin to unblock downstream work while the three PRs are in review; re-point to upstream revisions as they merge. The upstream PR (nimtable#163) stays pinned to #202's head, which resolves from the upstream repository and is the only form acceptable there.

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

…places Compaction materializes the deletes that were visible at `starting_snapshot_id` into the new data files, then replaces the originals. If another writer commits deletes for one of those originals in between, the new files do not contain them, and removing the old data file also retires the new delete file as dangling — so the rows it deleted come back. Pass `starting_snapshot_id` to the rewrite so the commit fails instead, which is what the existing "support validation of data files and delete files with starting snapshot" TODO asked for. Note the commit retry cannot recover from this: the closure reuses the already-computed output files and only reloads the table, so the conflict is permanent and the retries will exhaust before failing. That is the intended outcome — the compaction has to be planned again from the newer snapshot, which the caller does on its next run. Requires the `validate_from_snapshot_id` API from risingwavelabs/iceberg-rust#202, so the iceberg-rust rev must be bumped first.

vovacf201 · 2026-08-06T20:42:02Z

@chenzl25 friendly ping — all three points were addressed in 84b9670b, pushed the same day as the review, but the PR still shows Changes requested so it may not have resurfaced for you. Point-by-point, since my earlier reply was too terse to review against:

All three were correct. None were pushed back on.

1. Position deletes with referenced_data_file=None. Now attributed either via referenced_data_file or, failing that, by the same equal-bounds inference the scan side already performs. A new position delete that cannot be attributed at all is now treated as a conflict, since proving it harmless would require reading its contents.

There is a sub-finding here worth calling out: my original note describing that asymmetry as "fail-safe" was wrong, and in the more dangerous direction. For is_dangling_delete an unidentified target means the delete is kept, which is safe — but for validation it meant no conflict was reported and the commit proceeded. So the validation was strictly weaker than the reader, which is what your point 1 was pointing at.

2. Equality deletes / sequence-number prerequisite. The guarantee was real but conditional on a separate call the caller might simply not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so explicitly when it does not — so the prerequisite is enforced rather than assumed.

3. starting_snapshot_id ancestry. Previously only checked for existence. Now verified to be an ancestor of the branch head; a snapshot from unrelated lineage would have made every sequence-number comparison meaningless.

Tests. Six added, deliberately including the two must-not-over-reject directions — a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds — because all three fixes are easy to "pass" by simply rejecting more.

Each fix is mutation-verified. One of those checks caught a bad test: the ancestry test initially still passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

Happy to split any of the three into its own PR if that is easier to review than one commit. You also pinged @Li0k for a COW/branch perspective — that seems worth keeping, since point 3 is precisely about cross-branch lineage.

chenzl25 · 2026-08-10T06:50:18Z

@vovacf201 Thanks. I think this PR is worthwhile. Since we're trying to bump iceberg-rust (see #191), it might be better to merge this PR after we switch to 0807. What do you think?

chenzl25 · 2026-08-24T10:44:55Z

@vovacf201 We finished the branch switch to dev_rebase_main_20260807 now. Could you please migrate this PR to the new branch?

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

vovacf201 · 2026-08-26T10:30:23Z

Rebased onto dev_rebase_main_20260807. Both commits retained; validate_no_new_deletes_for_data_files is still absent from the new base.

Conflict resolutions worth a look:

transaction/replace_files.rs — the new base removed the merge_enabled / MergeManifestProcess branch from commit(). That branch was only diff context here, so the resolution keeps the base's single DefaultManifestProcess path and inserts the validate_no_new_deletes_for_data_files call ahead of it.
delete_file_index.rs — the new base independently added can_contain_pos_deletes_for_file plus its own POSITION_DELETE_FILE_PATH_FIELD_ID constant, colliding textually with this PR's try_infer_single_referenced_data_file_from_bounds and its duplicate constant. These are different operations (scan pruning vs. conflict attribution), so both functions are kept and the duplicate constant is dropped in favour of the base's.
transaction/snapshot.rs — Snapshot::load_manifest_list no longer exists; switched to table.manifest_list_reader(snapshot).load(), matching how validate_data_file_changes does it on the new base. This also routes the read through the encryption manager.
Test helpers — adapted to signature changes on the new base: ManifestWriterBuilder::new lost an argument, ManifestListWriter::v2 now takes Box<dyn FileWrite> rather than OutputFile, and SnapshotProducer::new lost an argument.

All 11 new validation tests pass. cargo fmt, cargo clippy --all-targets and all 1656 iceberg lib tests pass on each commit individually.

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

vovacf201 · 2026-08-27T10:03:04Z

merged with main

chenzl25 · 2026-08-27T11:02:00Z

@Li0k Please help to take a look, because we have some pruning optimization for COW table.

chenzl25 · 2026-08-27T11:04:01Z

Downstream integration note (especially for nimtable/iceberg-compaction#163 and RisingWave):

validate_from_snapshot_id is a same-branch rewrite guard: the validation snapshot must be an ancestor of the target branch, and replacement files must retain its sequence number.
Enable it for ordinary compaction, including the physical COW rewrite on the ingestion branch.
Do not automatically apply it to the COW publish step from ingestion to main. That step is an authoritative logical overwrite of main; the ingestion snapshot is not on the main lineage, so using it as the validation snapshot would reject valid publishes. Publish ordering or lease fencing, if required, belongs to the RisingWave scheduler and is separate from this delete-conflict check.
PK-index coordinated compaction relies on the Meta/resolver combined-commit protocol; this validation should not be used as a substitute for that coordination.

…places Compaction materializes the deletes that were visible at `starting_snapshot_id` into the new data files, then replaces the originals. If another writer commits deletes for one of those originals in between, the new files do not contain them, and removing the old data file also retires the new delete file as dangling — so the rows it deleted come back. Pass `starting_snapshot_id` to the rewrite so the commit fails instead, which is what the existing "support validation of data files and delete files with starting snapshot" TODO asked for. Note the commit retry cannot recover from this: the closure reuses the already-computed output files and only reloads the table, so the conflict is permanent and the retries will exhaust before failing. That is the intended outcome — the compaction has to be planned again from the newer snapshot, which the caller does on its next run. Requires the `validate_from_snapshot_id` API from risingwavelabs/iceberg-rust#202, so the iceberg-rust rev must be bumped first.

…eberg-rust#202 Not for merge as-is. This pins the `iceberg*` deps to `b087b307`, the head of risingwavelabs/iceberg-rust#202, which adds the `validate_from_snapshot_id` API the previous commit calls. Cargo resolves it because GitHub keeps a PR head reachable in the upstream repository, so CI can build and test the change while that PR is in review. Re-pin to the merge commit on `dev_rebase_main_20260303` once #202 lands, then this commit can be dropped or amended. All five `iceberg*` deps are moved together on purpose: pinning only one would pull in two incompatible copies of the `iceberg` crate.

…quence number Follow-up to the review of risingwavelabs/iceberg-rust#202, which hardened the validation: it now requires the replacement files to carry the starting snapshot's sequence number, because that is what keeps pre-existing equality deletes applicable to them. The `use_starting_sequence_number == false` path deliberately does not preserve it, so asking for validation there would just fail the commit. Drop the call on that branch and say why, rather than leave a guarantee that cannot hold. Concurrent-delete protection therefore requires `use_starting_sequence_number`. Also re-pin to the current head of #202 (84b9670b) so CI exercises the hardened validation.

… removes An operation that rewrites data files reads a snapshot, materializes the deletes that applied at that point into new data files, and then removes the originals. If another writer commits deletes for one of those originals in between, those deletes are never materialized — and because removing the data file also retires the new delete file as dangling, the rows it deleted silently come back. Nothing caught this. `validate_data_file_changes` only checks for duplicate adds and that the files being deleted exist, and the data file does still exist, so no conflict was raised. Add `SnapshotProducer::validate_no_new_deletes_for_data_files`, exposed on `ReplaceFilesAction` as `validate_from_snapshot_id`, which fails the commit when a delete file targeting one of the removed data files was added after the given snapshot. The caller can then retry against the current snapshot, where the new deletes are visible. Candidate manifests are narrowed by the manifest sequence number, then entries are filtered on their own sequence number, since a newer manifest can carry older entries forward as `Existing`. Only delete files that record `referenced_data_file` — deletion vectors, and position deletes written for a single data file — are covered, because that is the only case where the association is in the manifest. Position delete files spanning several data files would require reading their contents; equality deletes are retired by sequence number rather than by data file. Both are documented as out of scope. Opt-in, so existing callers are unaffected until they pass a starting snapshot.

Review of risingwavelabs#202 found the validation could still succeed while deletes were lost. All three points were correct. 1. Position deletes with no `referenced_data_file` were skipped even when their path bounds identify a single target. The scan side already infers this, so the validation was strictly weaker than the reader. Worse, the earlier note calling that asymmetry "fail-safe" was wrong: for `is_dangling_delete` an unidentified target means the delete is *kept*, which is safe, but for validation it means no conflict is reported and the commit proceeds. Now attributed via `referenced_data_file` or, failing that, the same equal-bounds inference. A new position delete that cannot be attributed at all is treated as a conflict, since proving it harmless would mean reading its contents. 2. Equality deletes are only safe because the replacement files keep the starting snapshot's sequence number, which kept them applicable — but nothing enforced that, so the guarantee was conditional on a separate call the caller might not make. Validation now requires the new data file sequence number to equal the starting snapshot's, and says so when it does not. 3. `starting_snapshot_id` was only checked for existence, not for being on the target branch. A snapshot from an unrelated lineage would have made every sequence-number comparison meaningless. It is now verified to be an ancestor of the branch head. Six tests added, including the must-not-over-reject directions: a bounds-identified delete for an untouched file, and a new equality delete once the prerequisite holds. Each fix is mutation-verified. The ancestry test initially passed with the ancestry check removed, because it accepted any error mentioning the snapshot id and the sequence-number error also mentions it. It now asserts the ancestry wording specifically.

…hot_id chenzl25's downstream integration note (for iceberg-compaction#163 and RisingWave) covered three things that were previously only in a PR comment, not in the API's own documentation: - it is a same-branch rewrite guard, safe for ordinary compaction and for a copy-on-write rewrite performed on its own branch - it must not be used to guard a copy-on-write *publish* from one branch into another -- that is an authoritative overwrite of the target, not a rewrite that read it, so the source branch's snapshot is never an ancestor of the target and comparing sequence numbers across that boundary is meaningless - it is not a substitute for out-of-band coordination between independent writers that must agree on a commit before either applies it (e.g. a combined-commit protocol with an external index) The ancestry check already enforces the first two mechanically, but the doc comment didn't explain why, which matters for anyone deciding whether to reach for this check versus real coordination. Added a "Scope" section to both the public entry point (ReplaceFilesAction::validate_from_snapshot_id) and the implementation it calls (SnapshotProducer::validate_no_new_deletes_for_data_files), kept in sync with each other. No behavior change. cargo fmt, cargo clippy --all-targets --all-features -- -D warnings, and cargo test -p iceberg --lib (transaction:: subset and full suite) all clean.

vovacf201 · 2026-09-03T10:27:52Z

Two things, per the request above:

Addressed the downstream integration note. That guidance (same-branch guard, safe for ordinary compaction / physical COW rewrite, not safe for the COW publish step, not a substitute for external write coordination) was valuable but only lived in a PR comment. Added a "Scope" section to both ReplaceFilesAction::validate_from_snapshot_id (the public entry point) and SnapshotProducer::validate_no_new_deletes_for_data_files (what it calls), kept in sync, so it's visible to anyone reading the API docs rather than needing to find this thread. No behavior change — the ancestry check already enforces the boundary mechanically, this just explains why, and names the COW-publish case explicitly so nobody "fixes" the ancestry check to allow it later.

Rebased onto the current dev_rebase_main_20260807 (8 commits ahead, including #212 which is why iceberg-compaction's own main pin already needs this branch — see nimtable/iceberg-compaction#163). All three commits (the original two plus the doc commit above) rebased with zero conflicts.

Verified on the final state: cargo check -p iceberg --all-targets --all-features, cargo test -p iceberg --lib → 1684 passed, cargo clippy -p iceberg --all-targets --all-features -- -D warnings clean, cargo fmt --check clean.

vovacf201 · 2026-09-03T11:19:41Z

Update: CI is now fully green on the rebased head (d44fe857) — all 19 checks pass, including test (windows-latest)/(macos-latest)/(ubuntu-latest), check-public-api, and clippy, which were still running when I posted the previous comment.

@chenzl25 one more thing worth flagging directly: the PR's review decision is still formally Changes Requested from your 2026-07-31 review. Your follow-up comment the same day ("Thanks. I think this PR is worthwhile...") reads like the three points were resolved, but that was a comment, not a review update, so GitHub is still showing this as blocked on it (mergeStateStatus: BLOCKED) even though there are no conflicts. Could you take another look and dismiss/re-review if it still looks right to you? I didn't want to dismiss your review myself.

chenzl25 · 2026-09-11T07:50:04Z

I will continue to review this PR when RisingWave's related iceberg features get merged into the main.

vovacf201 requested review from Li0k, agaddis02, chenzl25, mwylde, nagraham, netgusto and silver-ymz as code owners July 29, 2026 15:20

vovacf201 mentioned this pull request Jul 29, 2026

feat: fail the rewrite commit when deletes were added for files it replaces nimtable/iceberg-compaction#163

Open

chenzl25 requested changes Jul 31, 2026

View reviewed changes

vovacf201 requested a review from chenzl25 July 31, 2026 09:50

vovacf201 mentioned this pull request Aug 3, 2026

feat(spec): accept a v3 snapshot without a row range #203

Closed

vovacf201 force-pushed the u3a-c9 branch from 84b9670 to 3f782f3 Compare August 26, 2026 10:29

vovacf201 changed the base branch from dev_rebase_main_20260303 to dev_rebase_main_20260807 August 26, 2026 10:29

vovacf201 force-pushed the u3a-c9 branch from 3f782f3 to 02ab2a5 Compare August 27, 2026 09:50

vovacf201 added 3 commits September 3, 2026 11:25

vovacf201 force-pushed the u3a-c9 branch from 02ab2a5 to d44fe85 Compare September 3, 2026 10:27

chenzl25 mentioned this pull request Sep 17, 2026

perf(transaction): optimize manifest reads in RewriteFilesAction #238

Open

Navigation Menu

feat(transaction): detect concurrent deletes for data files a rewrite removes - #202

feat(transaction): detect concurrent deletes for data files a rewrite removes#202
vovacf201 wants to merge 3 commits into
risingwavelabs:dev_rebase_main_20260807from
vovacf201:u3a-c9

vovacf201 commented Jul 29, 2026

Uh oh!

chenzl25 commented Jul 31, 2026

Uh oh!

chenzl25 left a comment

Uh oh!

vovacf201 commented Jul 31, 2026

Uh oh!

vovacf201 commented Aug 6, 2026

Uh oh!

chenzl25 commented Aug 10, 2026

Uh oh!

chenzl25 commented Aug 24, 2026

Uh oh!

vovacf201 commented Aug 26, 2026

Uh oh!

vovacf201 commented Aug 27, 2026

Uh oh!

chenzl25 commented Aug 27, 2026

Uh oh!

chenzl25 commented Aug 27, 2026

Uh oh!

vovacf201 commented Sep 3, 2026

Uh oh!

vovacf201 commented Sep 3, 2026

Uh oh!

chenzl25 commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vovacf201 commented Jul 29, 2026

The bug

The fix

Implementation notes

Tests

Verification

Follow-up

Uh oh!

chenzl25 commented Jul 31, 2026

Uh oh!

chenzl25 left a comment

Choose a reason for hiding this comment

Uh oh!

vovacf201 commented Jul 31, 2026

Uh oh!

vovacf201 commented Aug 6, 2026

Uh oh!

chenzl25 commented Aug 10, 2026

Uh oh!

chenzl25 commented Aug 24, 2026

Uh oh!

vovacf201 commented Aug 26, 2026

Uh oh!

vovacf201 commented Aug 27, 2026

Uh oh!

chenzl25 commented Aug 27, 2026

Uh oh!

chenzl25 commented Aug 27, 2026

Uh oh!

vovacf201 commented Sep 3, 2026

Uh oh!

vovacf201 commented Sep 3, 2026

Uh oh!

chenzl25 commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants