raghav-reglobe commented on PR #2203: URL: https://github.com/apache/iceberg-rust/pull/2203#issuecomment-4861830099
Great to see this one moving again — we've been running this machinery in production for a few weeks and found something during review that I think is worth fixing here before merge. In `existing_manifest`'s rewrite loop, the carry-forward branch marks every non-removed entry as EXISTING without checking `is_alive()`. That means an entry that is already DELETED (superseded by an earlier rewrite) gets resurrected as live. On a table with some rewrite history this is easy to hit — each rewrite leaves one DELETED entry per affected data file — and for V3 deletion vectors the consequence is multiple live DVs pointing at one data file, which fails reads with "Can't index multiple DVs". Fresh test tables never catch it because they carry no DELETED entries yet; we only found it on real tables with longer histories. The fix is small: `else if entry.is_alive()` on the carry-forward, dropping already-DELETED entries (their deletion stays recorded in the originating snapshot's manifest). We just landed exactly this in #2678 (which carries your commits forward, authorship preserved) along with a regression test that constructs the deleted-then-rewritten shape — happy for you to lift both directly from there if useful: https://github.com/apache/iceberg-rust/pull/2678/commits/4e5802b79401d0bcebf3cdea25801e376e769efc Once this PR lands I'll rebase #2678 down to just the deletion-vector work on top of it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
