SreeramGarlapati opened a new pull request, #2511: URL: https://github.com/apache/iceberg-rust/pull/2511
## Which issue does this PR close? - Closes #2510. ## What changes are included in this PR? `update_snapshot_summaries` on the overwrite-with-truncate path was doing `truncate_table_summary(...).map_err(...).unwrap()`. The `unwrap()` turned a recoverable parse error into a process panic, and there are two ways to trip it from normal API usage. The first is a sneaky one: `get_prop` parsed the previous snapshot's totals as `i32`, even though every other counter path in the same file already uses `u64`. So once a table accumulated more than ~2.15B data files / records / bytes (i.e. crossed `i32::MAX`), the next overwrite-with-truncate would parse-fail, hit the unwrap, and crash the writer. Quietly tableable too — nothing in the public API hints that `i32` is the secret ceiling. The second is corruption / cross-implementation interop: any non-numeric value in a previous `total-*` property (a foreign implementation, a manual edit, a half-corrupt metadata file) produces a parse error and the same panic. The fix is small. Widen `get_prop` to `u64` so it matches the rest of the file and lifts the artificial ceiling. Replace the `.unwrap()` with `?`, keeping the existing `\"Failed to truncate table summary.\"` wrapper as caller context so the error chain still tells you what was being attempted when the inner parse blew up. `get_prop`'s error message now includes the offending property name and value, which is what you actually need when triaging a malformed metadata file. Scope-wise this only touches the overwrite-truncate path. `update_totals` has a structurally similar `unwrap()` antipattern that is a separate (lower-severity) issue and not included here. ## Are these changes tested? Yes — two unit tests in `snapshot_summary.rs`, both red-state proven against the unfixed code: The first asserts that a previous summary with `total-data-files = i32::MAX + 1` (and `total-records` similarly) survives the truncation and lands in `deleted-data-files` / `deleted-records` intact. Against the unfixed code this panics at `snapshot_summary.rs:356:18` with `\"number too large to fit in target type\"` — that's the i32-ceiling bug demonstrated directly. The second feeds `\"not_a_number\"` as a previous total and asserts the function returns an `Err` whose message carries the `\"truncate table summary\"` wrapper. Against the unfixed code this panics at the same site with `\"invalid digit found in string\"` — the malformed-input bug. I ran each test against the unfixed code, observed the panic, applied the fix, and watched them go green. Existing `test_truncate_table_summary` and `test_update_snapshot_summaries_append` continue to pass, and `cargo fmt --check` / `cargo clippy --all-features --tests -- -D warnings` are 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]
