fix: review comments and refactor transform test

Author: zhjwpku

src/iceberg/expression/literal.cc

@@ -337,31 +337,39 @@ 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) {
+std::partial_ordering CompareDecimal(Literal const& lhs, Literal const& rhs) {
   ICEBERG_DCHECK(std::holds_alternative<Decimal>(lhs.value()),
                  "LHS of decimal comparison must hold Decimal");
   ICEBERG_DCHECK(std::holds_alternative<Decimal>(rhs.value()),
                  "RHS of decimal comparison must hold decimal");
-  const auto& lhs_type = std::dynamic_pointer_cast<DecimalType>(lhs.type());
-  const auto& rhs_type = std::dynamic_pointer_cast<DecimalType>(rhs.type());
+  auto lhs_type = internal::checked_pointer_cast<DecimalType>(lhs.type());
+  auto rhs_type = internal::checked_pointer_cast<DecimalType>(rhs.type());
   auto lhs_decimal = std::get<Decimal>(lhs.value());
   auto rhs_decimal = std::get<Decimal>(rhs.value());
   if (lhs_type->scale() == rhs_type->scale()) {
     return lhs_decimal <=> rhs_decimal;
   } else if (lhs_type->scale() > rhs_type->scale()) {
     // Rescale to larger scale
     auto rhs_res = rhs_decimal.Rescale(rhs_type->scale(), lhs_type->scale());
-    if (!rhs_res) {
-      // Rescale would cause data loss, so lhs is definitely less than rhs
-      return std::strong_ordering::less;
+    if (!rhs_res.has_value()) {
+      if (rhs_res.error().kind == ErrorKind::kRescaleDataLoss) {
+        // Rescale would cause data loss, so lhs is definitely less than rhs
+        return std::partial_ordering::less;
+      }
+      // Other errors should return unordered
+      return std::partial_ordering::unordered;
     }
     return lhs_decimal <=> rhs_res.value();
   } else {
     // Rescale to larger scale
     auto lhs_res = lhs_decimal.Rescale(lhs_type->scale(), rhs_type->scale());
-    if (!lhs_res) {
-      // Rescale would cause data loss, so lhs is definitely greater than rhs
-      return std::strong_ordering::greater;
+    if (!lhs_res.has_value()) {
+      if (lhs_res.error().kind == ErrorKind::kRescaleDataLoss) {
+        // Rescale would cause data loss, so lhs is definitely greater than rhs
+        return std::partial_ordering::greater;
+      }
+      // Other errors should return unordered
+      return std::partial_ordering::unordered;
     }
     return lhs_res.value() <=> rhs_decimal;
   }

src/iceberg/result.h

@@ -47,6 +47,7 @@ enum class ErrorKind {
   kNotFound,
   kNotImplemented,
   kNotSupported,
+  kRescaleDataLoss,
   kUnknownError,
 };
 
@@ -95,6 +96,7 @@ DEFINE_ERROR_FUNCTION(NotAllowed)
 DEFINE_ERROR_FUNCTION(NotFound)
 DEFINE_ERROR_FUNCTION(NotImplemented)
 DEFINE_ERROR_FUNCTION(NotSupported)
+DEFINE_ERROR_FUNCTION(RescaleDataLoss)
 DEFINE_ERROR_FUNCTION(UnknownError)
 
 #undef DEFINE_ERROR_FUNCTION

src/iceberg/test/decimal_test.cc

@@ -712,7 +712,7 @@ TEST(DecimalTest, Rescale) {
   ASSERT_EQ(Decimal(5), Decimal(500000).Rescale(6, 1).value());
   ASSERT_EQ(Decimal(500000), Decimal(5).Rescale(1, 6).value());
 
-  ASSERT_THAT(Decimal(5555555).Rescale(6, 1), IsError(ErrorKind::kInvalid));
+  ASSERT_THAT(Decimal(5555555).Rescale(6, 1), IsError(ErrorKind::kRescaleDataLoss));
 }
 
 }  // namespace iceberg