fix truncate invalid max value && add comments for collect NaN

Author: WZhuo

src/iceberg/parquet/parquet_metrics.h

@@ -43,6 +43,7 @@ class ICEBERG_BUNDLE_EXPORT ParquetMetrics {
   ///
   /// This function extracts metrics including row count, column sizes, value counts,
   /// null value counts, and lower/upper bounds from Parquet file metadata.
+  /// NaN value counts are not currently collected from Parquet metadata.
   /// The metrics are computed according to the provided MetricsConfig, which determines
   /// which columns to collect metrics for and at what granularity (counts only, truncated
   /// bounds, or full bounds).

src/iceberg/parquet/parquet_writer.cc

@@ -164,6 +164,7 @@ class ParquetWriter::Impl {
     if (!metadata_) {
       return Metrics();
     }
+    // TODO(WZhuo): collect write-side FieldMetrics to support NaN value counts.
     return ParquetMetrics::GetMetrics(*schema_, *parquet_schema_, *metrics_config_,
                                       *metadata_, {});
   }

src/iceberg/test/truncate_util_test.cc

@@ -79,8 +79,9 @@ TEST(TruncateUtilTest, TruncateBinaryMax) {
   EXPECT_EQ(result3, Literal::Binary(test3));
 
   // Test3b: cannot truncate when first bytes are all 0xFF
-  EXPECT_THAT(TruncateUtils::TruncateLiteralMax(Literal::Binary(test3), 2),
-              IsError(ErrorKind::kInvalidArgument));
+  ICEBERG_UNWRAP_OR_FAIL(auto result3b,
+                         TruncateUtils::TruncateLiteralMax(Literal::Binary(test3), 2));
+  EXPECT_EQ(result3b, std::nullopt);
 
   // Test4: truncate {1, 1, 0} to 2 bytes -> {1, 2}
   ICEBERG_UNWRAP_OR_FAIL(auto result4,
@@ -143,8 +144,9 @@ TEST(TruncateUtilTest, TruncateStringMax) {
 
   // Test5: Max 4-byte UTF-8 characters "\uDBFF\uDFFF\uDBFF\uDFFF"
   std::string test5 = "\xF4\x8F\xBF\xBF\xF4\x8F\xBF\xBF";  // U+10FFFF U+10FFFF
-  EXPECT_THAT(TruncateUtils::TruncateLiteralMax(Literal::String(test5), 1),
-              IsError(ErrorKind::kInvalidArgument));
+  ICEBERG_UNWRAP_OR_FAIL(auto result5_1,
+                         TruncateUtils::TruncateLiteralMax(Literal::String(test5), 1));
+  EXPECT_EQ(result5_1, std::nullopt);
 
   // Test6: 4-byte UTF-8 character "\uD800\uDFFF\uD800\uDFFF"
   std::string test6 = "\xF0\x90\x8F\xBF\xF0\x90\x8F\xBF";  // U+103FF U+103FF

src/iceberg/util/truncate_util.cc

@@ -168,42 +168,46 @@ Literal TruncateLiteralImpl<TypeId::kBinary>(const Literal& literal, int32_t wid
 }
 
 template <TypeId type_id>
-Result<Literal> TruncateLiteralMaxImpl(const Literal& literal, int32_t width) = delete;
+Result<std::optional<Literal>> TruncateLiteralMaxImpl(const Literal& literal,
+                                                      int32_t width) = delete;
 
 template <>
-Result<Literal> TruncateLiteralMaxImpl<TypeId::kString>(const Literal& literal,
-                                                        int32_t width) {
+Result<std::optional<Literal>> TruncateLiteralMaxImpl<TypeId::kString>(
+    const Literal& literal, int32_t width) {
   const auto& str = std::get<std::string>(literal.value());
-  ICEBERG_ASSIGN_OR_RAISE(std::string truncated,
-                          TruncateUtils::TruncateUTF8Max(str, width));
-  return Literal::String(std::move(truncated));
+  ICEBERG_ASSIGN_OR_RAISE(auto truncated, TruncateUtils::TruncateUTF8Max(str, width));
+  if (!truncated.has_value()) {
+    return std::nullopt;
+  }
+  return std::optional<Literal>(Literal::String(std::move(truncated.value())));
 }
 
 template <>
-Result<Literal> TruncateLiteralMaxImpl<TypeId::kBinary>(const Literal& literal,
-                                                        int32_t width) {
+Result<std::optional<Literal>> TruncateLiteralMaxImpl<TypeId::kBinary>(
+    const Literal& literal, int32_t width) {
   const auto& data = std::get<std::vector<uint8_t>>(literal.value());
   if (static_cast<int32_t>(data.size()) <= width) {
-    return literal;
+    return std::optional<Literal>(literal);
   }
 
   std::vector<uint8_t> truncated(data.begin(), data.begin() + width);
   for (auto it = truncated.rbegin(); it != truncated.rend(); ++it) {
     if (*it < 0xFF) {
       ++(*it);
       truncated.resize(truncated.size() - std::distance(truncated.rbegin(), it));
-      return Literal::Binary(std::move(truncated));
+      return std::optional<Literal>(Literal::Binary(std::move(truncated)));
     }
   }
-  return InvalidArgument("Cannot truncate upper bound for binary: all bytes are 0xFF");
+  return std::nullopt;
 }
 
 }  // namespace
 
-Result<std::string> TruncateUtils::TruncateUTF8Max(const std::string& source, size_t L) {
+Result<std::optional<std::string>> TruncateUtils::TruncateUTF8Max(
+    const std::string& source, size_t L) {
   std::string truncated = TruncateUTF8(source, L);
   if (truncated == source) {
-    return truncated;
+    return std::optional<std::string>(std::move(truncated));
   }
 
   // Try incrementing code points from the end
@@ -232,13 +236,12 @@ Result<std::string> TruncateUtils::TruncateUTF8Max(const std::string& source, si
       if (next_code_point <= kUtf8MaxCodePoint) {
         truncated.resize(cp_start);
         AppendUtf8CodePoint(next_code_point, truncated);
-        return truncated;
+        return std::optional<std::string>(std::move(truncated));
       }
     }
     last_cp_start = cp_start;
   }
-  return InvalidArgument(
-      "Cannot truncate upper bound for string: all code points are 0x10FFFF");
+  return std::nullopt;
 }
 
 Decimal TruncateUtils::TruncateDecimal(const Decimal& decimal, int32_t width) {
@@ -275,10 +278,11 @@ Result<Literal> TruncateUtils::TruncateLiteral(const Literal& literal, int32_t w
   case TYPE_ID:                                \
     return TruncateLiteralMaxImpl<TYPE_ID>(literal, width);
 
-Result<Literal> TruncateUtils::TruncateLiteralMax(const Literal& literal, int32_t width) {
+Result<std::optional<Literal>> TruncateUtils::TruncateLiteralMax(const Literal& literal,
+                                                                 int32_t width) {
   if (literal.IsNull()) [[unlikely]] {
     // Return null as is
-    return literal;
+    return std::optional<Literal>(literal);
   }
 
   if (literal.IsAboveMax() || literal.IsBelowMin()) [[unlikely]] {
@@ -305,14 +309,14 @@ Result<Literal> TruncateUtils::TruncateLowerBound(const PrimitiveType& type,
   }
 }
 
-Result<Literal> TruncateUtils::TruncateUpperBound(const PrimitiveType& type,
-                                                  const Literal& value, int32_t length) {
+Result<std::optional<Literal>> TruncateUtils::TruncateUpperBound(
+    const PrimitiveType& type, const Literal& value, int32_t length) {
   switch (type.type_id()) {
     case TypeId::kString:
     case TypeId::kBinary:
       return TruncateLiteralMax(value, length);
     default:
-      return value;
+      return std::optional<Literal>(value);
   }
 }
 

src/iceberg/util/truncate_util.h

@@ -20,6 +20,7 @@
 #pragma once
 
 #include <cstdint>
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -71,9 +72,10 @@ class ICEBERG_EXPORT TruncateUtils {
   /// \param source The input string to truncate.
   /// \param L The maximum number of code points allowed in the output string.
   /// \return A Result containing the original string (if no truncation is
-  /// needed), or the smallest string greater than the truncated prefix, or an
-  /// error if no such value exists or the input is invalid UTF-8.
-  static Result<std::string> TruncateUTF8Max(const std::string& source, size_t L);
+  /// needed), the smallest string greater than the truncated prefix, or nullopt if no
+  /// safe upper bound can be represented. Invalid UTF-8 is returned as an error.
+  static Result<std::optional<std::string>> TruncateUTF8Max(const std::string& source,
+                                                            size_t L);
 
   /// \brief Truncate an integer v, either int32_t or int64_t, to v - (v % W).
   ///
@@ -109,10 +111,11 @@ class ICEBERG_EXPORT TruncateUtils {
   ///
   /// \param value The input Literal maximum value to truncate.
   /// \param width The width to truncate to.
-  /// \return A Result containing either the original Literal (if no truncation is needed)
-  /// or the smallest Literal greater than the truncated prefix, or an error if no such
-  /// value exists or cannot be represented.
-  static Result<Literal> TruncateLiteralMax(const Literal& value, int32_t width);
+  /// \return A Result containing either the original Literal (if no truncation is
+  /// needed), the smallest Literal greater than the truncated prefix, or nullopt if no
+  /// safe upper bound can be represented.
+  static Result<std::optional<Literal>> TruncateLiteralMax(const Literal& value,
+                                                           int32_t width);
 
   /// \brief Truncate the lower bound of a string or binary value.
   ///
@@ -134,9 +137,11 @@ class ICEBERG_EXPORT TruncateUtils {
   /// \param type The Iceberg primitive type.
   /// \param value The upper bound literal value.
   /// \param width The width to truncate to.
-  /// \return The truncated upper bound literal.
-  static Result<Literal> TruncateUpperBound(const PrimitiveType& type,
-                                            const Literal& value, int32_t width);
+  /// \return The truncated upper bound literal, or nullopt if no safe upper bound can be
+  /// represented.
+  static Result<std::optional<Literal>> TruncateUpperBound(const PrimitiveType& type,
+                                                           const Literal& value,
+                                                           int32_t width);
 };
 
 }  // namespace iceberg