feat: support PrecisionTimestamp and PrecisionTimestampTZ literals #283
Conversation
|
cc @vbarua |
|
Will look at this today. |
| var epochMicro = | ||
| TimeUnit.SECONDS.toMicros(value.toEpochSecond(ZoneOffset.UTC)) | ||
| + TimeUnit.NANOSECONDS.toMicros(value.toLocalTime().getNano()); | ||
| return precisionTimestamp(nullable, epochMicro, 6); |
There was a problem hiding this comment.
The docs for LocalDateTime indicate
Time is represented to nanosecond precision.
Should we be using nanoseconds (9) here instead of microseconds (6)?
There was a problem hiding this comment.
I have no real preference here - just kept it like this so that it aligns as closely as possible to the existing timestamp creators.
Given the LocalDate (and also Instant) contain more data than just a int64, (they have seconds and then the subseconds separately), I guess there's some dates that cannot be represented as only nanonseconds but can in microseconds. Quick search suggests that's about +- 292 years. So while we can switch this to nanoseconds, it could overflow for some inputs (if those inputs are like year 3000).
Frankly, I think I'd rather keep this as is, to make switching from the earlier timestamp behavior as easy as possible. If people want more control, they can always use the other creator func that lets them decide the precision. Alternatively, we could even consider not adding this second constructor. Thoughts?
There was a problem hiding this comment.
Alternatively, we could even consider not adding this second constructor.
Let's leave the new constructors out for now. It sounds like we may need to consider their behaviours more carefully now that we're not eliding to microseconds by default, but we don't need to wait to figure that out before merging the rest of this in.
| var epochMicro = | ||
| TimeUnit.SECONDS.toMicros(value.getEpochSecond()) | ||
| + TimeUnit.NANOSECONDS.toMicros(value.getNano()); | ||
| return precisionTimestampTZ(nullable, epochMicro, 6); |
There was a problem hiding this comment.
The docs for Instant indicate that
the class stores a long representing epoch-seconds and an int representing nanosecond-of-second
Should we be using nanoseconds (9) here instead of microseconds (6)?
Relatedly, are all possible Instant values representable as PrecistionTimestampTz literals?
There was a problem hiding this comment.
See the other answer. Not all instants are representable as PTTz, I believe, since instant stores in total 96 bits of data, while PTTz is only 64 bits.
No description provided.