u70b3 commented on PR #2185: URL: https://github.com/apache/iceberg-rust/pull/2185#issuecomment-5092896306
@timsaucer @c-thiel — following up on my review above (https://github.com/apache/iceberg-rust/pull/2185#issuecomment-4885503894): two bugs in the overwrite path, with fixes + regression tests at https://github.com/glitchy/iceberg-rust/pull/3: 1. `rewrite_manifest` rebuilds manifests from the table's current schema / default partition spec instead of the manifest's own — breaks after schema/partition evolution (spec L663/L741/L1010/L1053; Java keeps the manifest's own spec, `SnapshotProducer.java:632-639`) 2. Already-`Deleted` entries are flipped back to `Existing` on rewrite (Java drops them via `liveEntries()`, `ManifestFilterManager.java:509`) @glitchy has been inactive since June 30 (before I posted the review), so the fork PR has been sitting without a reviewer. How would the team prefer to proceed? 1. Wait for @glitchy to return and merge the fork PR — it would update this PR automatically; 2. A committer folds the fixes into this branch directly (maintainer pushes are enabled); 3. I carry the work in a new PR that preserves @glitchy's commits with these fixes on top. Happy to do (3) if there's agreement — this feature looks close, and it'd be a shame for it to stall on these two issues. (Separately, the branch now conflicts with main, so it needs a rebase in any case.) -- 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]
