Skip to content

Commit 26b1f1b

Browse files
committed
docs: clarify IsUtcOffset accepts Z/+00:00/-00:00; trim default-value comment
Addresses review feedback: document that any zero-offset spelling (Z, +00:00, -00:00) is accepted by TemporalUtils::IsUtcOffset, and shorten the verbose comment on the default-value members.
1 parent 0163d35 commit 26b1f1b

2 files changed

Lines changed: 9 additions & 7 deletions

File tree

src/iceberg/schema_field.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,7 @@ class ICEBERG_EXPORT SchemaField : public iceberg::util::Formattable {
134134
std::shared_ptr<Type> type_;
135135
bool optional_;
136136
std::string doc_;
137-
// Default values are owned by this field and never mutated after being set; copies
138-
// of the field share the same payload (reference-counted) instead of deep-copying,
139-
// like `type_` above. Sharing is unobservable because the payload is immutable.
137+
// Immutable default values, shared (not deep-copied) across field copies, like `type_`.
140138
std::shared_ptr<const Literal> initial_default_;
141139
std::shared_ptr<const Literal> write_default_;
142140
};

src/iceberg/util/temporal_util.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,19 @@ class ICEBERG_EXPORT TemporalUtils {
125125
/// \return The number of nanoseconds since epoch (UTC), or an error.
126126
static Result<int64_t> ParseTimestampNsWithZone(std::string_view str);
127127

128-
/// \brief Reports whether a timestamp-with-zone string uses a UTC offset.
128+
/// \brief Reports whether a timestamp-with-zone string carries a zero (UTC) offset.
129129
///
130130
/// The ParseTimestamp*WithZone parsers accept any offset and silently normalize it
131131
/// to UTC. The spec's JSON single-value form for `timestamptz` / `timestamptz_ns`
132-
/// default values only permits UTC ("Z" or "+00:00"), so callers that must enforce
133-
/// that rule check the offset here before parsing.
132+
/// default values is meant to be UTC, so callers that must enforce that rule check
133+
/// the offset here before parsing.
134+
///
135+
/// Any zero-offset spelling is treated as UTC: "Z", "+00:00" and "-00:00" all denote
136+
/// a zero offset and are accepted, even though the spec writes the canonical form as
137+
/// "+00:00". Only a genuinely non-zero offset (e.g. "+05:00") is rejected.
134138
///
135139
/// \param str The timestamp-with-zone string to inspect.
136-
/// \return true if the offset is UTC, false if it is a non-UTC offset, or an error
140+
/// \return true if the offset is zero, false if it is a non-zero offset, or an error
137141
/// if the timezone suffix cannot be parsed.
138142
static Result<bool> IsUtcOffset(std::string_view str);
139143

0 commit comments

Comments
 (0)