Skip to content

Commit b5531f2

Browse files
committed
refactor: RescaleWouldCauseDataLoss
1 parent 4c60d05 commit b5531f2

File tree

2 files changed

+27
-8
lines changed

2 files changed

+27
-8
lines changed

src/iceberg/test/decimal_test.cc

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -717,14 +717,24 @@ TEST(DecimalTest, Rescale) {
717717

718718
TEST(DecimalTest, Compare) {
719719
// max positive unscaled value
720-
ASSERT_EQ(Decimal::Compare(Decimal("170141183460469231731687303715884105727"),
721-
Decimal("170141183460469231731687303715884105727"), 2, 3),
720+
// 10^38 - 1 scale cause overflow
721+
ASSERT_EQ(Decimal::Compare(Decimal("99999999999999999999999999999999999999"),
722+
Decimal("99999999999999999999999999999999999999"), 2, 3),
722723
std::partial_ordering::greater);
724+
// 10^37 - 1 scale no overflow
725+
ASSERT_EQ(Decimal::Compare(Decimal("9999999999999999999999999999999999999"),
726+
Decimal("99999999999999999999999999999999999999"), 2, 3),
727+
std::partial_ordering::less);
723728

724729
// min negative unscaled value
725-
ASSERT_EQ(Decimal::Compare(Decimal("-170141183460469231731687303715884105728"),
726-
Decimal("-170141183460469231731687303715884105728"), 2, 3),
730+
// -10^38 + 1 scale cause overflow
731+
ASSERT_EQ(Decimal::Compare(Decimal("-99999999999999999999999999999999999999"),
732+
Decimal("-99999999999999999999999999999999999999"), 2, 3),
727733
std::partial_ordering::less);
734+
// -10^37 + 1 scale no overflow
735+
ASSERT_EQ(Decimal::Compare(Decimal("-9999999999999999999999999999999999999"),
736+
Decimal("-99999999999999999999999999999999999999"), 2, 3),
737+
std::partial_ordering::greater);
728738

729739
// equal values with different scales
730740
ASSERT_EQ(Decimal::Compare(Decimal("123456789"), Decimal("1234567890"), 2, 3),

src/iceberg/util/decimal.cc

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ namespace {
4747
static constexpr int32_t kMinDecimalBytes = 1;
4848
static constexpr int32_t kMaxDecimalBytes = 16;
4949

50+
// The maximum decimal value that can be represented with kMaxPrecision digits.
51+
// 10^38 - 1
52+
static constexpr Decimal kMaxDecimalValue(5421010862427522170LL, 687399551400673279ULL);
53+
// The mininum (most negative) decimal value that can be represented with kMaxPrecision
54+
// digits.
55+
// - (10^38 - 1)
56+
static constexpr Decimal kMinDecimalValue(-5421010862427522171LL,
57+
17759344522308878337ULL);
58+
5059
struct DecimalComponents {
5160
std::string_view while_digits;
5261
std::string_view fractional_digits;
@@ -278,15 +287,15 @@ bool RescaleWouldCauseDataLoss(const Decimal& value, int32_t delta_scale,
278287
return res->second != 0;
279288
}
280289

281-
int128_t max_safe_value = std::numeric_limits<int128_t>::max() / multiplier.value();
282-
int128_t min_safe_value = std::numeric_limits<int128_t>::min() / multiplier.value();
283-
if (value.value() > max_safe_value || value.value() < min_safe_value) {
290+
auto max_safe_value = kMaxDecimalValue / multiplier;
291+
auto min_safe_value = kMinDecimalValue / multiplier;
292+
if (value > max_safe_value || value < min_safe_value) {
284293
// Overflow would happen — treat as data loss
285294
return true;
286295
}
287296

288297
*result = value * multiplier;
289-
return (value < 0) ? *result > value : *result < value;
298+
return false;
290299
}
291300

292301
} // namespace

0 commit comments

Comments
 (0)