Add timestamp ns literal#3298
Conversation
|
@geruh @kevinjqliu found this while working on equality deletes + type promotion stuff. Should be pretty straight forward. ptal, thanks for your time! |
| @to.register(TimestampNanoType) | ||
| def _(self, _: TimestampNanoType) -> Literal[int]: | ||
| return TimestampNanoLiteral(self.value) | ||
|
|
||
| @to.register(TimestamptzNanoType) | ||
| def _(self, _: TimestamptzNanoType) -> Literal[int]: | ||
| return TimestampNanoLiteral(self.value) |
There was a problem hiding this comment.
Unfortunally, this has been shown to be controversial in the Iceberg-Java implementation: apache/iceberg#11775 (comment)
If you think about it in SQL: SELECT * FROM tbl WHERE dt > 1777883174. This assumes that the int is micros. However, when you would widen the field dt by converting it from micros to nanos, then the query would return a different result. When we would bind the LongLiteral to TimestampNanoLiteral we implicitly assume that it is a nanos after the widening of the column.
There was a problem hiding this comment.
I should've replied to this earlier! Sounds good. I'll close out this PR.
2 participants
Rationale for this change
Ran into this with the type promotion rabbit hole I've found myself in.
Turns out we don't have literal types for
timestamp_ns. This is definitely a missing part of being able to use timestamp_ns types.My goal here was to match the microsecond usage as much as possible.
(There's some weirdness for converting these to strings, since Python's string format doesn't actually handle nano-seconds for us. What fun!)
Are these changes tested?
Added some tests.
Are there any user-facing changes?