Abacn commented on code in PR #22561:
URL: https://github.com/apache/beam/pull/22561#discussion_r941777766
##########
sdks/python/apache_beam/typehints/schemas.py:
##########
@@ -647,9 +648,36 @@ def _from_typing(cls, typ):
('micros', np.int64)])
[email protected]_logical_type
+class DateTimeLogicalType(NoArgumentLogicalType[Timestamp, np.int64]):
Review Comment:
I agree this is preferable in terms of consistency. To do this we will need
to make distinction of timestamp in schema based IOs (including JDBC) and
windowing timestamp, making the former produce micros_instant instead of
joda.Instant, which is similar to the approach here #11456. However given the
scope of breaking changes this may leave as next major version change...
If I understood correctly it's not leaking Java's DATETIME primitive
(Schema.FieldType DATETIME), the schema passed to python is the logical type
`beam:logical_type:datetime:v1` that not yet implemented in python. However the
coder that used to decode this type is actually available in python which is
TimestampCoder. This is still in line with [Portable Beam Schema design
doc](https://docs.google.com/document/d/1uu9pJktzT_O3DxGd1-Q2op4nRk4HekIZbzi-0oTAips/edit#heading=h.w9oanj2p9iih).
--
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]
