PDGGK commented on PR #39804: URL: https://github.com/apache/beam/pull/39804#issuecomment-5330470151
That is a better framing than the one I brought, and I am happy to redo it that way. Having read through `Timestamp`, one thing about time-of-day does fall out differently, so let me put the concrete shape here before taking it to dev@. **Precision as an argument, same as `Timestamp`.** `getArgumentType()` = `INT32`, `getArgument()` = precision, `MIN_PRECISION = 0` / `MAX_PRECISION = 9`, plus `Time.of(int)` and `MILLIS` / `MICROS` / `NANOS` constants. **Representation is where it need not follow.** `Timestamp` uses `ROW<seconds: INT64, subseconds: INT16|INT32>` because epoch-seconds at nanosecond precision does not fit a single `INT64` — that overflows around 2262. Time-of-day is bounded: the largest value is 86400 × 10⁹ = 8.64 × 10¹³ nanoseconds, four orders of magnitude inside `Long.MAX_VALUE`. So a single `INT64` of sub-units since midnight is exact at every precision from 0 to 9, and the negative-modulo footgun that `Timestamp`'s javadoc has to warn about does not arise, because the domain has no negatives. The tradeoff is symmetry against simplicity: a `ROW` would let an SDK reuse whatever it wrote for `Timestamp`, while a plain `INT64` is less for each SDK to implement and needs no per-precision branching on the field width. My inclination is the `INT64`, but that is exactly the sort of thing worth other opinions. **One useful property either way.** With `INT64` sub-units since midnight, `Time.of(9)` is byte-identical to what the Java SDK produces today, so `SqlTypes.TIME` keeps its current representation and existing pipelines are unaffected — the precision parameter is purely additive. On Python: agreed that a custom type mirroring the `Timestamp` one is the way, rather than the `Date`-style approach in the follow-up I had planned. I will start the dev@ thread with the above and link back here. Meanwhile I will hold this PR rather than push a half-converted version — happy to close and reopen against whatever the list settles on, if you would prefer that to leaving it open. -- 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]
