fix(schema): align v3 default-value validation/serde with spec and Java

Addresses wgtmac's review feedback:

- Enforce UTC offset for timestamptz / timestamptz_ns default values in
  FieldFromJson. The shared timestamp parser accepts any offset and silently
  normalizes to UTC, so C++ could accept default metadata Java rejects and
  rewrite the offset on write. Added TemporalUtils::IsUtcOffset, which reuses
  the existing timezone-suffix parser.
- ValidateDefault now casts the default literal to the field type (Literal::CastTo)
  instead of requiring an exact type match, matching Java's Types.NestedField
  (e.g. an int default on a long field). Uncastable or out-of-range defaults are
  still rejected.
- Documented that non-primitive (struct/list/map) defaults remain unsupported,
  matching Java's current model.
- Extended the json_serde round-trip tests with time/timestamptz/timestamp_ns/
  timestamptz_ns and a negative case for non-UTC timestamptz defaults; added a
  schema test for the cast-to-field-type behavior.

Author: huan233usc

build-rest-tests.sh

@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+#
+# Build and run the REST catalog unit tests (target: rest_catalog_test, which
+# includes rest_json_serde_test.cc and rest_file_io_test.cc).
+#
+# Fast path: uses Homebrew LLVM (C++23 + libc++) and Homebrew apache-arrow, so
+# Arrow is found via find_package instead of being compiled from source.
+#
+# One-time install:
+#   brew install cmake ninja llvm apache-arrow
+#
+set -euo pipefail
+cd "$(dirname "$0")"
+
+LLVM_PREFIX="$(brew --prefix llvm)"
+export SDKROOT="${SDKROOT:-$(xcrun --show-sdk-path)}"   # help LLVM clang find the macOS SDK
+
+# NOTE: RelWithDebInfo (not Debug) on purpose. The vendored nanoarrow applies
+# `-Werror -Wpedantic` only in Debug ($<$<CONFIG:Debug>:...>), and Homebrew's
+# Clang 22 flags nanoarrow's use of `__COUNTER__` as a C2y extension, which then
+# becomes a fatal error. Non-Debug configs drop those flags.
+cmake -S . -B build -G Ninja \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_CXX_COMPILER="${LLVM_PREFIX}/bin/clang++" \
+  -DCMAKE_C_COMPILER="${LLVM_PREFIX}/bin/clang" \
+  -DCMAKE_PREFIX_PATH="$(brew --prefix apache-arrow);$(brew --prefix)" \
+  -DICEBERG_BUILD_REST=ON \
+  -DICEBERG_BUILD_TESTS=ON
+  # If the Homebrew Arrow version mismatches and fails to compile, force the
+  # vendored Arrow 24.0.0 build instead by adding:
+  #   -DFETCHCONTENT_TRY_FIND_PACKAGE_MODE=NEVER
+
+cmake --build build --target rest_catalog_test -j
+
+ctest --test-dir build -R rest_catalog_test --output-on-failure

src/iceberg/json_serde.cc

@@ -51,6 +51,7 @@
 #include "iceberg/util/json_util_internal.h"
 #include "iceberg/util/macros.h"
 #include "iceberg/util/string_util.h"
+#include "iceberg/util/temporal_util.h"
 #include "iceberg/util/timepoint.h"
 
 namespace iceberg {
@@ -564,6 +565,35 @@ Result<std::unique_ptr<Type>> TypeFromJson(const nlohmann::json& json) {
   }
 }
 
+namespace {
+
+// The spec's JSON single-value form for `timestamptz` / `timestamptz_ns` default
+// values requires a UTC offset. The shared timestamp parser accepts any offset and
+// silently normalizes to UTC, which would let C++ accept default metadata that Java
+// rejects and then rewrite the offset on serialization. Enforce UTC for these
+// defaults at parse time, where the original offset is still visible.
+Status ValidateTimestamptzDefaultIsUtc(const Type& type, const nlohmann::json& value) {
+  const auto type_id = type.type_id();
+  if (type_id != TypeId::kTimestampTz && type_id != TypeId::kTimestampTzNs) {
+    return {};
+  }
+  if (!value.is_string()) {
+    // Let LiteralFromJson report the type mismatch.
+    return {};
+  }
+  const auto str = value.get<std::string>();
+  ICEBERG_ASSIGN_OR_RAISE(bool is_utc, TemporalUtils::IsUtcOffset(str));
+  if (!is_utc) {
+    return JsonParseError(
+        "Invalid timestamptz default '{}' for {}: default values must use UTC "
+        "(offset 'Z' or '+00:00')",
+        str, type.ToString());
+  }
+  return {};
+}
+
+}  // namespace
+
 Result<std::unique_ptr<SchemaField>> FieldFromJson(const nlohmann::json& json) {
   ICEBERG_ASSIGN_OR_RAISE(
       auto type, GetJsonValue<nlohmann::json>(json, kType).and_then(TypeFromJson));
@@ -578,12 +608,16 @@ Result<std::unique_ptr<SchemaField>> FieldFromJson(const nlohmann::json& json) {
 
   std::shared_ptr<const Literal> initial_default;
   if (initial_default_json.has_value()) {
+    ICEBERG_RETURN_UNEXPECTED(
+        ValidateTimestamptzDefaultIsUtc(*type, *initial_default_json));
     ICEBERG_ASSIGN_OR_RAISE(Literal literal,
                             LiteralFromJson(*initial_default_json, type.get()));
     initial_default = std::make_shared<const Literal>(std::move(literal));
   }
   std::shared_ptr<const Literal> write_default;
   if (write_default_json.has_value()) {
+    ICEBERG_RETURN_UNEXPECTED(
+        ValidateTimestamptzDefaultIsUtc(*type, *write_default_json));
     ICEBERG_ASSIGN_OR_RAISE(Literal literal,
                             LiteralFromJson(*write_default_json, type.get()));
     write_default = std::make_shared<const Literal>(std::move(literal));

src/iceberg/schema_field.cc

@@ -93,14 +93,25 @@ Status ValidateDefault(const SchemaField& field, const Literal& value,
     return InvalidSchema("Invalid {} value for {}: must be a non-null value", kind,
                          field.name());
   }
+  // Defaults are only supported on primitive fields. The spec also permits JSON
+  // single-value defaults for struct/list/map (e.g. an empty struct `{}` whose
+  // sub-field defaults live in field metadata); that matches the current Java model's
+  // gap and is left as a follow-up.
   if (field.type() == nullptr || !field.type()->is_primitive()) {
     return InvalidSchema(
         "Invalid {} value for {}: default values are only supported for primitive types",
         kind, field.name());
   }
-  if (*value.type() != *field.type()) {
-    return InvalidSchema("{} of field {} has type {} but expected {}", kind, field.name(),
-                         *value.type(), *field.type());
+  // Match Java (Types.NestedField), which casts the default literal to the field type
+  // instead of requiring an exact type match (e.g. an int default on a long field, or
+  // a string default on a date/timestamp/uuid field). Reject only defaults that cannot
+  // be cast to the field type or fall outside its range (CastTo signals out-of-range as
+  // an above-max/below-min sentinel).
+  auto field_type = std::static_pointer_cast<PrimitiveType>(field.type());
+  auto cast = value.CastTo(field_type);
+  if (!cast.has_value() || cast->IsAboveMax() || cast->IsBelowMin()) {
+    return InvalidSchema("{} of field {} has type {} that cannot be cast to {}", kind,
+                         field.name(), *value.type(), *field.type());
   }
   return {};
 }

src/iceberg/test/json_serde_test.cc

@@ -136,7 +136,11 @@ TEST(JsonInternalTest, SchemaFieldDefaultValuesRoundTripAllTypes) {
   cases.emplace_back(float32(), Literal::Float(1.5f));
   cases.emplace_back(float64(), Literal::Double(2.5));
   cases.emplace_back(date(), Literal::Date(19738));
+  cases.emplace_back(time(), Literal::Time(43200000000LL));
   cases.emplace_back(timestamp(), Literal::Timestamp(1719446400000000LL));
+  cases.emplace_back(timestamp_tz(), Literal::TimestampTz(1719446400000000LL));
+  cases.emplace_back(timestamp_ns(), Literal::TimestampNs(1719446400000000123LL));
+  cases.emplace_back(timestamptz_ns(), Literal::TimestampTzNs(1719446400000000123LL));
   cases.emplace_back(string(), Literal::String("hello"));
   cases.emplace_back(decimal(9, 2), Literal::Decimal(12345, 9, 2));
   cases.emplace_back(fixed(3), Literal::Fixed({0x01, 0x02, 0x03}));
@@ -155,6 +159,23 @@ TEST(JsonInternalTest, SchemaFieldDefaultValuesRoundTripAllTypes) {
   }
 }
 
+// The spec only permits UTC offsets for timestamptz / timestamptz_ns default values.
+// A non-UTC offset (which the shared parser would silently normalize) must be rejected,
+// while the UTC form is accepted.
+TEST(JsonInternalTest, SchemaFieldRejectsNonUtcTimestamptzDefault) {
+  auto non_utc = nlohmann::json::parse(
+      R"({"id":1,"name":"ts","required":true,"type":"timestamptz","initial-default":"2024-06-27T05:00:00+05:00"})");
+  EXPECT_FALSE(FieldFromJson(non_utc).has_value());
+
+  auto non_utc_ns = nlohmann::json::parse(
+      R"({"id":1,"name":"ts","required":true,"type":"timestamptz_ns","write-default":"2024-06-27T05:00:00-08:00"})");
+  EXPECT_FALSE(FieldFromJson(non_utc_ns).has_value());
+
+  auto utc = nlohmann::json::parse(
+      R"({"id":1,"name":"ts","required":true,"type":"timestamptz","initial-default":"2024-06-27T00:00:00+00:00"})");
+  EXPECT_TRUE(FieldFromJson(utc).has_value());
+}
+
 TEST(JsonInternalTest, SortField) {
   auto identity_transform = Transform::Identity();
 

src/iceberg/test/schema_test.cc

@@ -167,6 +167,23 @@ TEST(SchemaTest, ValidateRejectsMismatchedDefaultValue) {
   EXPECT_THAT(status, iceberg::HasErrorMessage("write-default"));
 }
 
+TEST(SchemaTest, ValidateCastsDefaultToFieldType) {
+  // Matching Java, the default literal is cast to the field type rather than required to
+  // match exactly: an int default on a long field is accepted (int -> long).
+  iceberg::Schema widened({iceberg::SchemaField(
+      1, "id", iceberg::int64(), false, /*doc=*/{},
+      std::make_shared<const iceberg::Literal>(iceberg::Literal::Int(34)))});
+  EXPECT_THAT(widened.Validate(iceberg::TableMetadata::kSupportedTableFormatVersion),
+              iceberg::IsOk());
+
+  // A default whose type cannot be cast to the field type is still rejected.
+  iceberg::Schema uncastable({iceberg::SchemaField(
+      1, "id", iceberg::int32(), false, /*doc=*/{},
+      std::make_shared<const iceberg::Literal>(iceberg::Literal::String("oops")))});
+  EXPECT_THAT(uncastable.Validate(iceberg::TableMetadata::kSupportedTableFormatVersion),
+              iceberg::IsError(iceberg::ErrorKind::kInvalidSchema));
+}
+
 TEST(SchemaTest, ReassignIdsPreservesDefaultValues) {
   // Reassigning field IDs rebuilds each SchemaField, so the rebuild must carry the
   // default values over to the field with the new ID.

src/iceberg/util/temporal_util.cc

@@ -444,6 +444,11 @@ Result<int64_t> TemporalUtils::ParseTimestampNsWithZone(std::string_view str) {
                               /*units_per_micro=*/internal::kNanosPerMicro);
 }
 
+Result<bool> TemporalUtils::IsUtcOffset(std::string_view str) {
+  ICEBERG_ASSIGN_OR_RAISE(auto timestamp_with_offset, ParseTimestampWithZoneSuffix(str));
+  return timestamp_with_offset.second == 0;
+}
+
 #define DISPATCH_EXTRACT_YEAR(type_id) \
   case type_id:                        \
     return ExtractYearImpl<type_id>(literal);

src/iceberg/util/temporal_util.h

@@ -125,6 +125,18 @@ class ICEBERG_EXPORT TemporalUtils {
   /// \return The number of nanoseconds since epoch (UTC), or an error.
   static Result<int64_t> ParseTimestampNsWithZone(std::string_view str);
 
+  /// \brief Reports whether a timestamp-with-zone string uses a UTC offset.
+  ///
+  /// The ParseTimestamp*WithZone parsers accept any offset and silently normalize it
+  /// to UTC. The spec's JSON single-value form for `timestamptz` / `timestamptz_ns`
+  /// default values only permits UTC ("Z" or "+00:00"), so callers that must enforce
+  /// that rule check the offset here before parsing.
+  ///
+  /// \param str The timestamp-with-zone string to inspect.
+  /// \return true if the offset is UTC, false if it is a non-UTC offset, or an error
+  ///         if the timezone suffix cannot be parsed.
+  static Result<bool> IsUtcOffset(std::string_view str);
+
   /// \brief Extract a date or timestamp year, as years from 1970
   static Result<Literal> ExtractYear(const Literal& literal);