fix: review comments

Author: zhjwpku

src/iceberg/expression/literal.cc

@@ -134,6 +134,8 @@ Literal Literal::Date(int32_t value) { return {Value{value}, iceberg::date()}; }
 
 Literal Literal::Long(int64_t value) { return {Value{value}, iceberg::int64()}; }
 
+Literal Literal::Time(int64_t value) { return {Value{value}, iceberg::time()}; }
+
 Literal Literal::Timestamp(int64_t value) { return {Value{value}, iceberg::timestamp()}; }
 
 Literal Literal::TimestampTz(int64_t value) {
@@ -210,30 +212,15 @@ std::partial_ordering Literal::operator<=>(const Literal& other) const {
       return this_val ? std::partial_ordering::greater : std::partial_ordering::less;
     }
 
-    case TypeId::kInt: {
-      auto this_val = std::get<int32_t>(value_);
-      auto other_val = std::get<int32_t>(other.value_);
-      return this_val <=> other_val;
-    }
-
+    case TypeId::kInt:
     case TypeId::kDate: {
       auto this_val = std::get<int32_t>(value_);
       auto other_val = std::get<int32_t>(other.value_);
       return this_val <=> other_val;
     }
 
-    case TypeId::kLong: {
-      auto this_val = std::get<int64_t>(value_);
-      auto other_val = std::get<int64_t>(other.value_);
-      return this_val <=> other_val;
-    }
-
-    case TypeId::kTimestamp: {
-      auto this_val = std::get<int64_t>(value_);
-      auto other_val = std::get<int64_t>(other.value_);
-      return this_val <=> other_val;
-    }
-
+    case TypeId::kLong:
+    case TypeId::kTimestamp:
     case TypeId::kTimestampTz: {
       auto this_val = std::get<int64_t>(value_);
       auto other_val = std::get<int64_t>(other.value_);

src/iceberg/expression/literal.h

@@ -49,6 +49,7 @@ class ICEBERG_EXPORT Literal {
     std::strong_ordering operator<=>(const AboveMax&) const = default;
   };
 
+ public:
   using Value = std::variant<bool,         // for boolean
                              int32_t,      // for int, date
                              int64_t,      // for long, timestamp, timestamp_tz, time
@@ -59,12 +60,12 @@ class ICEBERG_EXPORT Literal {
                              std::array<uint8_t, 16>,  // for uuid and decimal
                              BelowMin, AboveMax>;
 
- public:
   /// \brief Factory methods for primitive types
   static Literal Boolean(bool value);
   static Literal Int(int32_t value);
   static Literal Date(int32_t value);
   static Literal Long(int64_t value);
+  static Literal Time(int64_t value);
   static Literal Timestamp(int64_t value);
   static Literal TimestampTz(int64_t value);
   static Literal Float(float value);

src/iceberg/manifest_reader_internal.cc

@@ -33,6 +33,11 @@
 
 namespace iceberg {
 
+#define NANOARROW_RETURN_IF_NOT_OK(status, error)                  \
+  if (status != NANOARROW_OK) [[unlikely]] {                       \
+    return InvalidArrowData("Nanoarrow error: {}", error.message); \
+  }
+
 #define PARSE_PRIMITIVE_FIELD(item, array_view, type)                                   \
   for (size_t row_idx = 0; row_idx < array_view->length; row_idx++) {                   \
     if (!ArrowArrayViewIsNull(array_view, row_idx)) {                                   \

src/iceberg/util/macros.h

@@ -36,8 +36,3 @@
 #define ICEBERG_ASSIGN_OR_RAISE(lhs, rexpr)                                             \
   ICEBERG_ASSIGN_OR_RAISE_IMPL(ICEBERG_ASSIGN_OR_RAISE_NAME(result_, __COUNTER__), lhs, \
                                rexpr)
-
-#define NANOARROW_RETURN_IF_NOT_OK(status, error)                  \
-  if (status != NANOARROW_OK) [[unlikely]] {                       \
-    return InvalidArrowData("Nanoarrow error: {}", error.message); \
-  }