huan233usc commented on code in PR #746:
URL: https://github.com/apache/iceberg-cpp/pull/746#discussion_r3488580982
##########
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));
Review Comment:
Thanks for raising this — I dug into both the spec and the reference
implementation, and I think accepting `Z` / `+00:00` / `-00:00` is correct here.
**What the spec says:** default values use the single-value JSON
serialization (Appendix D). For `timestamptz` / `timestamptz_ns` the value must
be **UTC**, serialized as an ISO-8601 timestamp with a `+00:00` offset (e.g.
`2017-11-16T22:31:08.123456+00:00`). The requirement is that the offset is UTC;
`Z` and `-00:00` are just equivalent zero-offset spellings of the same instant.
**What the reference impl does:** `SingleValueParser` gates timestamptz
defaults on `DateTimeUtil.isUTCTimestamptz`, which is:
```java
OffsetDateTime odt = OffsetDateTime.parse(s,
DateTimeFormatter.ISO_DATE_TIME);
return odt.getOffset().equals(ZoneOffset.UTC);
```
So it checks the parsed offset equals UTC — `Z`, `+00:00` and `-00:00` all
pass, and only a genuinely non-zero offset (e.g. `+05:00`) is rejected. That's
exactly what `IsUtcOffset` does here. (Java's error string reads "offset must
be +00:00", but the underlying check is offset == UTC.)
Refs:
- Spec, Appendix D — Single-value serialization:
https://iceberg.apache.org/spec/#appendix-d-single-value-serialization
- `DateTimeUtil.isUTCTimestamptz`:
https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/util/DateTimeUtil.java
Happy to tighten it to `+00:00`-only if you'd prefer, though that would
reject UTC values the Java reference accepts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
