wgtmac commented on code in PR #653:
URL: https://github.com/apache/iceberg-cpp/pull/653#discussion_r3289699351
##########
src/iceberg/type_fwd.h:
##########
@@ -46,6 +46,8 @@ enum class TypeId {
kTime,
kTimestamp,
kTimestampTz,
+ kTimestampNs,
Review Comment:
This needs the same v3 gate as Java. Otherwise a v2 table can accept and
write a v3-only schema.
##########
src/iceberg/util/transform_util.cc:
##########
@@ -208,32 +312,32 @@ Result<int64_t>
TransformUtil::ParseTimestamp(std::string_view str) {
return static_cast<int64_t>(days) * kMicrosPerDay + time_micros;
}
-Result<int64_t> TransformUtil::ParseTimestampWithZone(std::string_view str) {
- if (str.empty()) [[unlikely]] {
- return InvalidArgument("Invalid timestamptz string: '{}'", str);
+Result<int64_t> TransformUtil::ParseTimestampNs(std::string_view str) {
+ auto t_pos = str.find('T');
+ if (t_pos == std::string_view::npos) [[unlikely]] {
+ return InvalidArgument("Invalid timestamp string (missing 'T'): '{}'",
str);
}
- int64_t offset_micros = 0;
- std::string_view timestamp_part;
+ ICEBERG_ASSIGN_OR_RAISE(auto days, ParseDay(str.substr(0, t_pos)));
+ ICEBERG_ASSIGN_OR_RAISE(auto time_nanos, ParseTimeNs(str.substr(t_pos + 1)));
- if (str.back() == 'Z') {
- // "Z" suffix means UTC (offset = 0)
- timestamp_part = str.substr(0, str.size() - 1);
- } else if (str.size() >= 6 &&
- (str[str.size() - 6] == '+' || str[str.size() - 6] == '-')) {
- // Parse "+HH:mm" or "-HH:mm" offset suffix
- ICEBERG_ASSIGN_OR_RAISE(offset_micros,
- ParseTimezoneOffset(str.substr(str.size() - 6)));
- timestamp_part = str.substr(0, str.size() - 6);
- } else {
- return InvalidArgument("Invalid timestamptz string (missing timezone
suffix): '{}'",
- str);
- }
+ return static_cast<int64_t>(days) * kNanosPerDay + time_nanos;
Review Comment:
This can overflow past the int64 nanos boundary. Please use checked
arithmetic here and in the timezone offset path.
--
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]
