ITADN

bug(spec): update_snapshot_summaries panics on overwrite when previous totals exceed i32::MAX or are non-numeric

#2510OpenSreeramGarlapati 创建于 2026-05-26
### Apache Iceberg Rust version `main` (verified against `a3d2764`). ### Describe the bug `update_snapshot_summaries` in `crates/iceberg/src/spec/snapshot_summary.rs` panics on the overwrite-truncation path whenever the previous snapshot's summary contains a total that doesn't fit `i32`, or any value that isn't a decimal integer. The crash site is line 356: ```rust truncate_table_summary(summary, prev_summary) .map_err(|err| { Error::new(ErrorKind::Unexpected, "Failed to truncate table summary.") .with_source(err) }) .unwrap() // <-- panics on Err ``` `truncate_table_summary` calls `get_prop` for every `total-*` property, and `get_prop` parses with `parse::<i32>()`. Two failure modes: 1. **Capacity**: `i32::MAX` is ~2.15 billion. Any table whose previous snapshot has `total-data-files`, `total-records`, `total-files-size`, etc. above that ceiling produces `Err(number too large to fit in target type)`. The rest of the file already parses these counters as `u64`, so `get_prop` is the odd one out. 2. **Validation**: any non-numeric value (corruption, manual metadata edits, a value written by a foreign implementation) produces `Err(invalid digit found in string)`. Both errors are wrapped into a `Result::Err` and then `.unwrap()`-ed, which is a process-level panic — not a recoverable error a caller can handle. ### Impact - Any overwrite-with-truncate against a table large enough to cross the `i32` ceiling crashes the writer process. - Any overwrite against a table whose summary was last touched by another implementation crashes the writer process. - The crash is on a code path that's reachable by normal API usage (`update_snapshot_summaries(.., truncate_full_table=true)` from the overwrite transaction), not on an internal-only path. ### To Reproduce ```rust let big = (i32::MAX as u64 + 1).to_string(); // 2_147_483_648 let previous_summary = Summary { operation: Operation::Overwrite, additional_properties: HashMap::from([ (\"total-data-files\".to_string(), big.clone()), // ... other totals zero-valued ]), }; let summary = Summary { operation: Operation::Overwrite, additional_properties: HashMap::new(), }; // Today: panics at snapshot_summary.rs:356:18 with // \"Result::unwrap() on an Err value: ... number too large to fit in target type\" // Expected: returns Ok with deleted-data-files = \"2147483648\". update_snapshot_summaries(summary, Some(&previous_summary), true).unwrap(); ``` Same shape with `\"not_a_number\"` instead of `big` — same panic, different inner error message. ### Expected behaviour - Totals up to `u64::MAX` should be accepted (consistent with how `update_totals` already treats them). - Malformed values should produce a recoverable `Err` (wrapped with the existing `\"Failed to truncate table summary.\"` context), not a panic. ### Willingness to contribute Yes — fix and red-state-proven tests prepared, PR incoming.
0 条评论