diff --git a/src/iceberg/test/truncate_util_test.cc b/src/iceberg/test/truncate_util_test.cc index 10093e440..42a42acc2 100644 --- a/src/iceberg/test/truncate_util_test.cc +++ b/src/iceberg/test/truncate_util_test.cc @@ -19,6 +19,9 @@ #include "iceberg/util/truncate_util.h" +#include +#include + #include #include "iceberg/expression/literal.h" @@ -51,6 +54,54 @@ TEST(TruncateUtilTest, TruncateLiteral) { Literal::Binary(std::vector(expected.begin(), expected.end()))); } +TEST(TruncateUtilTest, TruncateIntegerBoundaryValues) { + auto check_boundaries = [](auto make_literal) { + const T min = std::numeric_limits::min(); + const T max = std::numeric_limits::max(); + struct TestCase { + T value; + int32_t width; + T expected; + }; + std::vector cases = { + {min, 1, min}, + {-1, 1, -1}, + {0, 1, 0}, + {1, 1, 1}, + {max, 1, max}, + // The final subtraction wraps for the type minimum, matching Java. + {min, 10, max - 1}, + {max, 10, max - max % 10}, + // Values just above the type minimum truncate without wrapping. + {min + 8, 10, min + 8}, + {min + 9, 10, min + 8}, + // Java's truncateInt overflows (v % W) + W here; follow v - (v % W) + // instead, yielding 0 for both types, as Java's truncateLong also does. + {2147483645, 2147483646, 0}, + }; + for (int32_t width : {10, std::numeric_limits::max() - 1, + std::numeric_limits::max()}) { + const T w = width; + cases.insert(cases.end(), {{0, width, 0}, + {1, width, 0}, + {w - 1, width, 0}, + {-1, width, -w}, + {1 - w, width, -w}, + {w, width, w}, + {-w, width, -w}}); + } + + for (const auto& [value, width, expected] : cases) { + SCOPED_TRACE(::testing::Message() << "value=" << value << ", width=" << width); + EXPECT_EQ(TruncateUtils::TruncateInteger(value, width), expected); + EXPECT_EQ(TruncateUtils::TruncateLiteral(make_literal(value), width), + make_literal(expected)); + } + }; + check_boundaries.operator()(Literal::Int); + check_boundaries.operator()(Literal::Long); +} + TEST(TruncateUtilTest, TruncateLiteralRejectsInvalidWidth) { std::vector data{1, 2, 3}; diff --git a/src/iceberg/util/truncate_util.h b/src/iceberg/util/truncate_util.h index 0ed686546..80c58e85f 100644 --- a/src/iceberg/util/truncate_util.h +++ b/src/iceberg/util/truncate_util.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include "iceberg/iceberg_export.h" @@ -82,12 +83,20 @@ class ICEBERG_EXPORT TruncateUtils { /// \brief Truncate an integer v, either int32_t or int64_t, to v - (v % W). /// - /// The remainder, v % W, must be positive. For languages where % can produce negative - /// values, the correct truncate function is: v - (((v % W) + W) % W) + /// W must be positive. Since C++ can produce a negative remainder, add W only + /// when v % W is negative to normalize it to [0, W). This addition cannot overflow. + /// The final subtraction uses unsigned arithmetic to avoid signed-overflow undefined + /// behavior when the truncated value is below T's minimum. Conversion back to T + /// preserves two's-complement wraparound (defined since C++20). template requires std::is_same_v || std::is_same_v static inline T TruncateInteger(T v, int32_t W) { - return v - (((v % W) + W) % W); + using U = std::make_unsigned_t; + T remainder = v % W; + if (remainder < 0) { + remainder += W; + } + return static_cast(static_cast(v) - static_cast(remainder)); } /// \brief Truncate a Decimal to a specified width.