SreeramGarlapati opened a new issue, #2510:
URL: https://github.com/apache/iceberg-rust/issues/2510
### 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.
--
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]
