[dragonknightk]

Which issue does this PR close?

• Closes Truncate transform diverges from Iceberg Java's TruncateUtil for widths above 2^30, and panics in debug builds at the integer minimum #3141.

What changes are included in this PR?

truncate_i32 used rem_euclid, while truncate_i64 and truncate_decimal_i128 used (v % w + w) % w. Both shapes differ from Iceberg Java's TruncateUtil, which relies on int/long overflow wrapping:

• For truncate_i32, rem_euclid never overflows, so any width above 2^30 produced a different partition value than Java.
• For the other two, the final subtraction overflowed at the integer minimum, panicking in debug builds and wrapping to the Java value only in release builds.

All three helpers now use wrapping_sub and wrapping_add, so every width reproduces the Java result in both debug and release builds. Each helper has a comment noting that the formula intentionally matches Java, so it is not "simplified" back to rem_euclid later. The function signatures, the public API, and the call sites are unchanged.

Are these changes tested?

Three unit tests in crates/iceberg/src/transform/truncate.rs. All expected values were computed with Iceberg Java's TruncateUtil (1.11.0, JDK 17):

• test_truncate_int_matches_java covers the three i32 cases from the issue, plus the smallest width where (v % w) + w can overflow (2^30 + 1) and a tiny value at the maximum width. Each case is checked through both transform on an array and transform_literal.
• test_truncate_long_min_matches_java covers i64::MIN with widths 1000 and i32::MAX, through the same two paths.
• test_truncate_decimal_i128_min_does_not_panic pins that the decimal path does not panic at i128::MIN. That value is outside the range of a 38-digit Iceberg decimal, and Java's truncateDecimal works on BigInteger, so there is no wrapping result to compare against.

Reverting the helpers to their previous bodies makes all three tests fail. For the i64 cases the old formula happens to give the Java values once wrapped, so it is caught by the debug-build overflow panic.

AI Disclosure

Human-led contribution. Codex and Grok Bot, working for dragonknightk, assisted with this change. All submitted content was reviewed by a human before submission.

[dragonknightk]

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 (｡•̀ᴗ-)✧

[Baunsgaard]

Thanks for this. The three helpers now use the same wrapping formula as Java
TruncateUtil, I left a few nits for various details.

[Baunsgaard]

nit: v.wrapping_sub((v % width).wrapping_add(width) % width) reads like a
bad v - v.rem_euclid(width), which is exactly the bug this PR undoes.

One comment above would keep someone from "simplifying" it later:

// Match Java TruncateUtil, including int/long overflow wrapping.

[Baunsgaard]

add comment

[Baunsgaard]

add comment

[Baunsgaard]

nit: the name test_truncate_long_min_does_not_panic sounds like the decimal test, which only checks that debug does not crash and then throws the result away. This test does more.

Something like test_truncate_long_min_matches_java matches what it asserts.

I would also extend it to do like the int test, to iterate a few cases:

for (width, value, expected) in [
    (1000, i64::MIN, 9_223_372_036_854_775_616),
    (i32::MAX as u32, i64::MIN, 9_223_372_034_707_292_163),
] {
    let truncate = super::Truncate::new(width);
    let input = Arc::new(Int64Array::from(vec![Some(value), None]));
    let result = truncate.transform(input).unwrap();
    let expected_array = Int64Array::from(vec![Some(expected), None]);
    assert_eq!(
        result.as_any().downcast_ref::<Int64Array>(),
        Some(&expected_array)
    );
}

[Baunsgaard]

nit: i would consider removing the wrapping_arithmetic part of the name.

[Baunsgaard]

other fun ones:

// Smallest width where (v % w) + w can overflow i32.
(1_073_741_825, 1_073_741_824, 2_147_483_646), // 2^30+1 ; rem_euclid was 0
// Tiny value, max width: Java wraps to 2, rem_euclid was 0.
(i32::MAX as u32, 1, 2),

[dragonknightk]

Thanks for the careful review, @Baunsgaard! All the nits are addressed in the updated commit ʕ•ᴥ•ʔ

• Added the "Match Java TruncateUtil" comment above all three helpers. For the decimal one I worded it slightly differently, since Java's truncateDecimal works on BigInteger and never wraps, so the comment there says the wrapping only keeps out-of-range i128 values from panicking.
• Renamed the long test to test_truncate_long_min_matches_java and turned it into a loop over your two cases. I also kept the transform_literal check so it lines up with the int test.
• Dropped wrapping_arithmetic from the int test name, so it's now test_truncate_int_matches_java.
• Added your two extra int cases. I double-checked all the new expected values against TruncateUtil from iceberg 1.11.0 on JDK 17, and they match (๑•̀ㅂ•́)و✧