minor fixes

Author: wgtmac

src/iceberg/CMakeLists.txt

@@ -108,6 +108,7 @@ set(ICEBERG_SOURCES
     util/murmurhash3_internal.cc
     util/property_util.cc
     util/snapshot_util.cc
+    util/string_util.cc
     util/temporal_util.cc
     util/timepoint.cc
     util/transform_util.cc

src/iceberg/expression/json_serde.cc

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -315,11 +316,18 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
       }
       return Literal::Boolean(json.get<bool>());
 
-    case TypeId::kInt:
+    case TypeId::kInt: {
       if (!json.is_number_integer()) [[unlikely]] {
         return JsonParseError("Cannot parse {} as an int value", SafeDumpJson(json));
       }
-      return Literal::Int(json.get<int32_t>());
+      auto val = json.get<int64_t>();
+      if (val < std::numeric_limits<int32_t>::min() ||
+          val > std::numeric_limits<int32_t>::max()) [[unlikely]] {
+        return JsonParseError("Cannot parse {} as an int value: out of range",
+                              SafeDumpJson(json));
+      }
+      return Literal::Int(static_cast<int32_t>(val));
+    }
 
     case TypeId::kLong:
       if (!json.is_number_integer()) [[unlikely]] {
@@ -328,13 +336,13 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
       return Literal::Long(json.get<int64_t>());
 
     case TypeId::kFloat:
-      if (!json.is_number()) [[unlikely]] {
+      if (!json.is_number_float()) [[unlikely]] {
         return JsonParseError("Cannot parse {} as a float value", SafeDumpJson(json));
       }
       return Literal::Float(json.get<float>());
 
     case TypeId::kDouble:
-      if (!json.is_number()) [[unlikely]] {
+      if (!json.is_number_float()) [[unlikely]] {
         return JsonParseError("Cannot parse {} as a double value", SafeDumpJson(json));
       }
       return Literal::Double(json.get<double>());
@@ -418,12 +426,15 @@ Result<Literal> LiteralFromJson(const nlohmann::json& json, const Type* type) {
         return JsonParseError("Cannot parse {} as a decimal value", SafeDumpJson(json));
       }
       const auto& dec_type = internal::checked_cast<const DecimalType&>(*type);
+      int32_t parsed_precision = 0;
       int32_t parsed_scale = 0;
       ICEBERG_ASSIGN_OR_RAISE(
-          auto dec, Decimal::FromString(json.get<std::string>(), nullptr, &parsed_scale));
-      if (parsed_scale != dec_type.scale()) [[unlikely]] {
-        return JsonParseError("Cannot parse {} as a {} value: the scale doesn't match",
-                              SafeDumpJson(json), type->ToString());
+          auto dec,
+          Decimal::FromString(json.get<std::string>(), &parsed_precision, &parsed_scale));
+      if (parsed_precision > dec_type.precision() || parsed_scale != dec_type.scale())
+          [[unlikely]] {
+        return JsonParseError("Cannot parse {} as a {} value", SafeDumpJson(json),
+                              type->ToString());
       }
       return Literal::Decimal(dec.value(), dec_type.precision(), dec_type.scale());
     }

src/iceberg/expression/literal.cc

@@ -220,17 +220,24 @@ Result<Literal> LiteralCaster::CastFromString(
       return Literal::Binary(std::move(bytes));
     }
     case TypeId::kFixed: {
+      const auto& fixed_type = internal::checked_cast<const FixedType&>(*target_type);
+      if (str_val.size() != static_cast<size_t>(fixed_type.length()) * 2) {
+        return InvalidArgument("Cannot cast string to {}: expected {} hex chars, got {}",
+                               target_type->ToString(), fixed_type.length() * 2,
+                               str_val.size());
+      }
       ICEBERG_ASSIGN_OR_RAISE(auto bytes, StringUtils::HexStringToBytes(str_val));
       return Literal::Fixed(std::move(bytes));
     }
     case TypeId::kDecimal: {
       const auto& dec_type = internal::checked_cast<const DecimalType&>(*target_type);
+      int32_t parsed_precision = 0;
       int32_t parsed_scale = 0;
-      ICEBERG_ASSIGN_OR_RAISE(auto dec,
-                              Decimal::FromString(str_val, nullptr, &parsed_scale));
-      if (parsed_scale != dec_type.scale()) {
-        return InvalidArgument("Cannot cast {} as a {} value: the scale doesn't match",
-                               str_val, target_type->ToString());
+      ICEBERG_ASSIGN_OR_RAISE(
+          auto dec, Decimal::FromString(str_val, &parsed_precision, &parsed_scale));
+      if (parsed_precision > dec_type.precision() || parsed_scale != dec_type.scale()) {
+        return InvalidArgument("Cannot cast {} as a {} value", str_val,
+                               target_type->ToString());
       }
       return Literal::Decimal(dec.value(), dec_type.precision(), dec_type.scale());
     }

src/iceberg/meson.build

@@ -126,6 +126,7 @@ iceberg_sources = files(
     'util/murmurhash3_internal.cc',
     'util/property_util.cc',
     'util/snapshot_util.cc',
+    'util/string_util.cc',
     'util/temporal_util.cc',
     'util/timepoint.cc',
     'util/transform_util.cc',

src/iceberg/test/expression_json_test.cc

@@ -424,7 +424,9 @@ TEST_P(LiteralFromJsonTypedTest, Parses) {
   const auto& p = GetParam();
   ICEBERG_UNWRAP_OR_FAIL(auto lit, LiteralFromJson(p.json, p.type.get()));
   EXPECT_EQ(lit.type()->type_id(), p.expected_type_id);
-  if (p.expected_str) EXPECT_EQ(lit.ToString(), *p.expected_str);
+  if (p.expected_str) {
+    EXPECT_EQ(lit.ToString(), *p.expected_str);
+  }
 }
 
 INSTANTIATE_TEST_SUITE_P(
@@ -493,13 +495,41 @@ INSTANTIATE_TEST_SUITE_P(
       return info.param.name;
     });
 
-TEST(LiteralFromJsonTyped, SchemaAwareDatePredicateRoundTrip) {
+struct SchemaAwarePredicateParam {
+  std::string name;
+  std::string field_name;
+  std::shared_ptr<Type> field_type;
+  nlohmann::json value;
+};
+
+class SchemaAwarePredicateRoundTripTest
+    : public ::testing::TestWithParam<SchemaAwarePredicateParam> {};
+
+TEST_P(SchemaAwarePredicateRoundTripTest, RoundTrip) {
+  const auto& p = GetParam();
   auto schema = std::make_shared<Schema>(
-      std::vector<SchemaField>{SchemaField::MakeOptional(1, "event_date", date())});
-  nlohmann::json pred_json = {
-      {"type", "eq"}, {"term", "event_date"}, {"value", "2024-01-15"}};
+      std::vector<SchemaField>{SchemaField::MakeOptional(1, p.field_name, p.field_type)});
+  nlohmann::json pred_json = {{"type", "eq"}, {"term", p.field_name}, {"value", p.value}};
   ICEBERG_UNWRAP_OR_FAIL(auto expr, ExpressionFromJson(pred_json, schema.get()));
   ASSERT_NE(expr, nullptr);
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    LiteralFromJsonTyped, SchemaAwarePredicateRoundTripTest,
+    ::testing::Values(
+        SchemaAwarePredicateParam{"Date", "event_date", date(), "2024-01-15"},
+        SchemaAwarePredicateParam{"Time", "event_time", time(), "14:30:00"},
+        SchemaAwarePredicateParam{"Timestamp", "created_at", timestamp(),
+                                  "2026-01-01T00:00:01.500"},
+        SchemaAwarePredicateParam{"TimestampTz", "updated_at", timestamp_tz(),
+                                  "2026-01-01T00:00:01.500+00:00"},
+        SchemaAwarePredicateParam{"Uuid", "trace_id", uuid(),
+                                  "f79c3e09-677c-4bbd-a479-3f349cb785e7"},
+        SchemaAwarePredicateParam{"Binary", "payload", binary(), "deadbeef"},
+        SchemaAwarePredicateParam{"Fixed", "hash", fixed(4), "cafebabe"},
+        SchemaAwarePredicateParam{"Decimal", "amount", decimal(9, 2), "123.45"}),
+    [](const ::testing::TestParamInfo<SchemaAwarePredicateParam>& info) {
+      return info.param.name;
+    });
+
 }  // namespace iceberg

src/iceberg/util/string_util.cc

@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/util/string_util.h"
+
+#include "iceberg/util/macros.h"
+
+namespace iceberg {
+
+Result<std::vector<uint8_t>> StringUtils::HexStringToBytes(std::string_view hex) {
+  if (hex.size() % 2 != 0) [[unlikely]] {
+    return InvalidArgument("Hex string must have even length, got: {}", hex.size());
+  }
+  std::vector<uint8_t> bytes;
+  bytes.reserve(hex.size() / 2);
+  auto nibble = [](char c) -> Result<uint8_t> {
+    if (c >= '0' && c <= '9') return static_cast<uint8_t>(c - '0');
+    if (c >= 'a' && c <= 'f') return static_cast<uint8_t>(c - 'a' + 10);
+    if (c >= 'A' && c <= 'F') return static_cast<uint8_t>(c - 'A' + 10);
+    return InvalidArgument("Invalid hex character: '{}'", c);
+  };
+  for (size_t i = 0; i < hex.size(); i += 2) {
+    ICEBERG_ASSIGN_OR_RAISE(auto hi, nibble(hex[i]));
+    ICEBERG_ASSIGN_OR_RAISE(auto lo, nibble(hex[i + 1]));
+    bytes.push_back(static_cast<uint8_t>((hi << 4) | lo));
+  }
+  return bytes;
+}
+
+}  // namespace iceberg

src/iceberg/util/string_util.h

@@ -32,7 +32,6 @@
 
 #include "iceberg/iceberg_export.h"
 #include "iceberg/result.h"
-#include "iceberg/util/macros.h"
 
 namespace iceberg {
 
@@ -80,6 +79,10 @@ class ICEBERG_EXPORT StringUtils {
     T value = 0;
     auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value);
     if (ec == std::errc()) [[likely]] {
+      if (ptr != str.data() + str.size()) {
+        return InvalidArgument("Failed to parse {} from string '{}': trailing characters",
+                               typeid(T).name(), str);
+      }
       return value;
     }
     if (ec == std::errc::invalid_argument) {
@@ -95,25 +98,7 @@ class ICEBERG_EXPORT StringUtils {
 
   /// \brief Decode a hex string (upper or lower case) into bytes.
   /// Returns an error if the string has odd length or contains invalid hex characters.
-  static Result<std::vector<uint8_t>> HexStringToBytes(std::string_view hex) {
-    if (hex.size() % 2 != 0) [[unlikely]] {
-      return InvalidArgument("Hex string must have even length, got: {}", hex.size());
-    }
-    std::vector<uint8_t> bytes;
-    bytes.reserve(hex.size() / 2);
-    auto nibble = [](char c) -> Result<uint8_t> {
-      if (c >= '0' && c <= '9') return static_cast<uint8_t>(c - '0');
-      if (c >= 'a' && c <= 'f') return static_cast<uint8_t>(c - 'a' + 10);
-      if (c >= 'A' && c <= 'F') return static_cast<uint8_t>(c - 'A' + 10);
-      return InvalidArgument("Invalid hex character: '{}'", c);
-    };
-    for (size_t i = 0; i < hex.size(); i += 2) {
-      ICEBERG_ASSIGN_OR_RAISE(auto hi, nibble(hex[i]));
-      ICEBERG_ASSIGN_OR_RAISE(auto lo, nibble(hex[i + 1]));
-      bytes.push_back(static_cast<uint8_t>((hi << 4) | lo));
-    }
-    return bytes;
-  }
+  static Result<std::vector<uint8_t>> HexStringToBytes(std::string_view hex);
 
   template <typename T>
     requires std::is_floating_point_v<T> && (!FromChars<T>)

src/iceberg/util/transform_util.cc

@@ -187,7 +187,7 @@ Result<int64_t> TransformUtil::ParseTime(std::string_view str) {
 
   // check that hours, minutes, seconds are in valid ranges
   if (hours < 0 || hours > 23 || minutes < 0 || minutes > 59 || seconds < 0 ||
-      seconds > 60) [[unlikely]] {
+      seconds > 59) [[unlikely]] {
     return InvalidArgument("Invalid time string: '{}'", str);
   }
 

src/iceberg/util/transform_util.h

@@ -109,15 +109,19 @@ class ICEBERG_EXPORT TransformUtil {
 
   /// \brief Parses a time string into microseconds from midnight.
   ///
-  /// Accepts: "HH:mm", "HH:mm:ss", "HH:mm:ss.SSS", "HH:mm:ss.SSSSSS".
+  /// Accepts ISO-8601 local time formats: "HH:mm", "HH:mm:ss", or
+  /// "HH:mm:ss.f" where the fractional part can be 1-9 digits.
+  /// Digits beyond 6 (microsecond precision) are truncated.
   ///
   /// \param str The time string to parse.
   /// \return The number of microseconds from midnight, or an error.
   static Result<int64_t> ParseTime(std::string_view str);
 
   /// \brief Parses a timestamp string into microseconds since epoch.
   ///
-  /// Accepts: "yyyy-MM-ddTHH:mm:ss", with optional fractional seconds (.SSS or .SSSSSS).
+  /// Accepts ISO-8601 local date-time formats: "yyyy-MM-ddTHH:mm",
+  /// "yyyy-MM-ddTHH:mm:ss", or "yyyy-MM-ddTHH:mm:ss.f" where the
+  /// fractional part can be 1-9 digits (truncated to microseconds).
   ///
   /// \param str The timestamp string to parse.
   /// \return The number of microseconds since epoch, or an error.
@@ -127,6 +131,7 @@ class ICEBERG_EXPORT TransformUtil {
   ///
   /// Accepts the same formats as ParseTimestamp, with a timezone suffix:
   /// "Z", "+HH:mm", or "-HH:mm". Non-UTC offsets are converted to UTC.
+  /// The seconds and fractional parts are optional (e.g. "yyyy-MM-ddTHH:mm+00:00").
   ///
   /// \param str The timestamp string to parse.
   /// \return The number of microseconds since epoch (UTC), or an error.