dragonknightk commented on PR #3573: URL: https://github.com/apache/iceberg-rust/pull/3573#issuecomment-6487105988
CI is green across ubuntu, macos and windows. Could I get a review when someone has a moment? (´｡• ᵕ •｡`) @kevinjqliu you offered to take a look at #3141 back in October, so tagging you here in case you still want to — no rush at all ✧ Summary of the change: all three helpers in `truncate.rs` now use `wrapping_sub` / `wrapping_add` instead of the previous shapes, so the `i32` path matches Java for widths above 2^30 and the `i64`/`i128` paths no longer panic at the integer minimum in debug builds. The expected values in the new tests come from `TruncateUtil` on iceberg 1.11.0 / JDK 17, and reverting the fix makes all three fail. @andygrove thanks for the detailed report on #3141 — the tables in it made this straightforward to verify. I followed the `wrapping_sub` / `wrapping_add` shape you suggested, so both problems in the issue are covered by the same change, and the expected values are the ones you listed (｡•̀ᴗ-)✧ -- 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]
