viirya opened a new pull request, #2726:
URL: https://github.com/apache/iceberg-rust/pull/2726
## Which issue does this PR close?
- Closes #2725.
## What changes are included in this PR?
Building a snapshot summary let a user-supplied snapshot property overwrite
a computed metric key, and a non-integer value then panicked total computation.
Both are fixed to match iceberg-java.
**1. Computed metrics now win over user properties
(`transaction/snapshot.rs`).** Previously:
```rust
let mut additional_properties = summary_collector.build();
additional_properties.extend(self.snapshot_properties.clone()); // user
overwrites computed
```
Now the user properties are applied first and the computed metrics overwrite
them:
```rust
let mut additional_properties = self.snapshot_properties.clone();
additional_properties.extend(summary_collector.build()); //
computed overwrites user
```
This matches `SnapshotSummary.Builder.build()` in iceberg-java, which does
`builder.putAll(properties)` (custom props) first and then
`metrics.addTo(builder)`, so computed metrics win on a key collision.
**2. `update_totals` tolerates an unparseable delta instead of panicking
(`spec/snapshot_summary.rs`).** The `added-*`/`removed-*` parses used
`.expect("must be parsable as it was just serialized")`. A non-integer value
(which can also legitimately come from a previous snapshot's summary) panicked
the commit. It now skips that total, matching iceberg-java's
`SnapshotProducer.updateTotal`, which wraps the `Long.parseLong` calls in `try
{ … } catch (NumberFormatException e) { // ignore and do not add total }`.
Both Java behaviors verified against `apache/iceberg` `main`
(`core/src/main/java/org/apache/iceberg/SnapshotProducer.java`,
`SnapshotSummary.java`).
## Are these changes tested?
Two new regression tests:
-
`transaction::append::test_snapshot_properties_cannot_override_computed_metrics`
— a fast-append whose `snapshot_properties` set `added-data-files=9999` and
`added-records=<non-integer>` commits without panicking, and the resulting
summary reports the real computed counts (`1`), not the user values.
-
`spec::snapshot_summary::test_update_totals_tolerates_unparseable_added_value`
— an unparseable `added-*` value skips that one total while sibling totals
still compute, with no panic.
Both fail if the respective fix is reverted (verified). Full `iceberg` lib
suite (1373 tests) passes, clippy + rustfmt clean.
--
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]
