rexminnis commented on PR #3046: URL: https://github.com/apache/iceberg-rust/pull/3046#issuecomment-5874223407
Thanks for working through all of it, and thanks @jopdorp for the fixes. The spec-evolution fixture is up as hotcache/iceberg-rust#2, two commits on top of `e7db3c8`: - **`test(transaction): cover rewrites on a spec-evolved table`**: a table partitioned by `identity(ts)`, then evolved to `day(ts)`, run on V2 and V3. A survivor stays `Existing` in a spec-0 manifest, with its timestamptz partition value and that manifest's partition summary unchanged. One rewrite filters one manifest of each spec. Retiring the last old-spec file drops the spec-0 manifest, and the totals stay exact across both rewrites. All three pass on your branch as it is, so the spec lookup in the filter holds up. - **`fix(transaction): keep partition metrics in the rewrite summary`**: the fixture found one gap. `build_summary` merges the filter's removal collector into a new one, and `SnapshotSummaryCollector::merge` drops partition metrics unless both sides trust them. Neither side does, because `Default` leaves `trust_partition_metrics` false. The result is that a replace snapshot never reports `changed-partition-count` or `partitions.*`. A fast append on the same table reports `changed-partition-count=1`, and the rewrite reports nothing. The fix adds the new files to the filter's collector instead of merging, and honours `write.summary.partition-limit` the same way `SnapshotProducer::summary` does. With the fix, the rewrite reports `ts=2026-08-24+09%3A30%3A00+UTC` for the removed file and `ts_day=2026-08-24` for the added one, each named under its own spec. The new test fails without the fix. (Java's `SnapshotSummary.Builder` starts with `trustPartitionMetrics = true`. The Rust default is the other way round, and `snapshot_summary.rs` has a test that asserts merging two default collectors drops the partition summaries. So I kept the fix local to the rewrite rather than changing the shared default. That default may deserve its own issue.) `cargo test -p iceberg --lib` passes 1817 tests, and `cargo fmt --check` and `clippy -D warnings` are clean. No public API changes. The REST-catalog run with an independent reader is next. I'll report back here once it's done. -- 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]
