feat: literal decimal compare & add more test cases

Author: zhjwpku

src/iceberg/expression/literal.cc

@@ -344,6 +344,42 @@ std::strong_ordering CompareFloat(T lhs, T rhs) {
   return lhs_is_negative <=> rhs_is_negative;
 }
 
+std::strong_ordering CompareDecimal(Literal const& lhs, Literal const& rhs) {
+  ICEBERG_DCHECK(std::holds_alternative<int128_t>(lhs.value()),
+                 "LHS of decimal comparison must hold int128_t");
+  ICEBERG_DCHECK(std::holds_alternative<int128_t>(rhs.value()),
+                 "RHS of decimal comparison must hold int128_t");
+  const auto& lhs_type = std::dynamic_pointer_cast<DecimalType>(lhs.type());
+  const auto& rhs_type = std::dynamic_pointer_cast<DecimalType>(rhs.type());
+  ICEBERG_DCHECK(lhs_type != nullptr, "LHS type must be DecimalType");
+  ICEBERG_DCHECK(rhs_type != nullptr, "RHS type must be DecimalType");
+  auto lhs_val = std::get<int128_t>(lhs.value());
+  auto rhs_val = std::get<int128_t>(rhs.value());
+  if (lhs_type->scale() == rhs_type->scale()) {
+    return lhs_val <=> rhs_val;
+  } else if (lhs_type->scale() > rhs_type->scale()) {
+    auto lhs_decimal = Decimal(lhs_val);
+    // Rescale to larger scale
+    auto rhs_decimal = Decimal(rhs_val).Rescale(rhs_type->scale(), lhs_type->scale());
+    if (!rhs_decimal) {
+      // Rescale would cause data loss, so lhs is definitely less than rhs
+      return std::strong_ordering::less;
+    }
+    return lhs_decimal <=> rhs_decimal.value();
+  } else {
+    auto rhs_decimal = Decimal(rhs_val);
+    // Rescale to larger scale
+    auto lhs_decimal = Decimal(lhs_val).Rescale(lhs_type->scale(), rhs_type->scale());
+    if (!lhs_decimal) {
+      // Rescale would cause data loss, so lhs is definitely greater than rhs
+      return std::strong_ordering::greater;
+    }
+    return lhs_decimal.value() <=> rhs_decimal;
+  }
+
+  return lhs_val <=> rhs_val;
+}
+
 bool Literal::operator==(const Literal& other) const { return (*this <=> other) == 0; }
 
 // Three-way comparison operator
@@ -410,10 +446,7 @@ std::partial_ordering Literal::operator<=>(const Literal& other) const {
     }
 
     case TypeId::kDecimal: {
-      // TODO(zhjwpku): Handle precision/scale differences
-      auto this_val = std::get<int128_t>(value_);
-      auto other_val = std::get<int128_t>(other.value_);
-      return this_val <=> other_val;
+      return CompareDecimal(*this, other);
     }
 
     default:

test/decimal_test.cc

@@ -502,6 +502,7 @@ TEST(DecimalTest, ToBigEndian) {
                                       INT64_MIN};
   std::vector<uint64_t> low_values = {0,
                                       1,
+                                      255,
                                       UINT32_MAX,
                                       static_cast<uint64_t>(UINT32_MAX) + 1,
                                       static_cast<uint64_t>(UINT32_MAX) + 2,
@@ -522,6 +523,15 @@ TEST(DecimalTest, ToBigEndian) {
       EXPECT_EQ(result.value(), value);
     }
   }
+
+  for (int128_t value : std::vector<int128_t>{-INT64_MAX, -INT32_MAX, -255, -1, 0, 1, 255,
+                                              256, INT32_MAX, INT64_MAX}) {
+    Decimal decimal(value);
+    auto bytes = Decimal::ToBigEndian(decimal.value());
+    auto result = Decimal::FromBigEndian(bytes.data(), bytes.size());
+    ASSERT_THAT(result, IsOk());
+    EXPECT_EQ(result.value(), decimal);
+  }
 }
 
 TEST(DecimalTestFunctionality, Multiply) {

test/literal_test.cc

@@ -384,6 +384,24 @@ TEST(LiteralTest, DoubleZeroComparison) {
   EXPECT_EQ(neg_zero <=> pos_zero, std::partial_ordering::less);
 }
 
+TEST(LiteralTest, DecimalComparison) {
+  auto dec1 = Literal::Decimal("123.45");
+  auto dec2 = Literal::Decimal("123.450");
+  auto dec3 = Literal::Decimal("123.46");
+  auto dec4 = Literal::Decimal("-123.45");
+
+  ASSERT_THAT(dec1, IsOk());
+  ASSERT_THAT(dec2, IsOk());
+  ASSERT_THAT(dec3, IsOk());
+  ASSERT_THAT(dec4, IsOk());
+
+  EXPECT_EQ((*dec1 <=> *dec2), std::partial_ordering::equivalent);
+  EXPECT_EQ((*dec1 <=> *dec3), std::partial_ordering::less);
+  EXPECT_EQ((*dec3 <=> *dec1), std::partial_ordering::greater);
+  EXPECT_EQ((*dec1 <=> *dec4), std::partial_ordering::greater);
+  EXPECT_EQ((*dec4 <=> *dec1), std::partial_ordering::less);
+}
+
 TEST(LiteralTest, SerdeTest) {
   // int32
   auto int_literal = Literal::Int(42);