JFinis commented on code in PR #8658:
URL: https://github.com/apache/iceberg/pull/8658#discussion_r1342463104
##########
api/src/main/java/org/apache/iceberg/types/Types.java:
##########
@@ -48,6 +48,8 @@ private Types() {}
.put(TimeType.get().toString(), TimeType.get())
.put(TimestampType.withZone().toString(), TimestampType.withZone())
.put(TimestampType.withoutZone().toString(),
TimestampType.withoutZone())
+ .put(TimestampnsType.withZone().toString(),
TimestampnsType.withZone())
Review Comment:
Thanks, now I see the discussions! That being said, I don't see a discussion
that discusses whether a new type should be used vs. a new field in the
timestamp type. Can you point me to that discussion? Maybe I'm missing the
forest for the trees here. Or the discussion was removed due to the text to
which it refers to being removed?
I see that it was a suggestion in your old versions. Therefore, I would like
to understand the rationale of the person who was against it.
> > the design was to separate different timestamps through fields, not new
types
> Help me out, where do you read that?
I read that in the code itself: There is only one `timestamp` type with an
`adjust-to-utc` flag. Instead, there could have been two types, `timestamp` and
`timestamptz`. Thus, the person doing the initial design decided for a flag
instead of two different types.
And now you add another dimension in which timestamps can be different.
Contrarily, the design closest to the initial design would be to have yet
another (enum) field on the timestamp type instead of a new type. This would
also help with the clumsy backward compatible naming of `timestamp` (without
any suffix) and now `timestamp_ns`, where one type has a suffix while the other
one hasn't. With a new field, everything would just stay `timestamp` and the
field would implicitly be Millis, if it's not set.
So at least for me, this points towards having the field. Though, maybe
there is a disadvantage of the field approach I don't see here.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
