flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] wuchong edited a comment on issue #10035: [FLINK-14080][table-planner-blink] Introduce Timestamp as internal representation of TIMESTAMP_WITHOUT_TIME_ZONE
Date Thu, 31 Oct 2019 03:54:24 GMT
wuchong edited a comment on issue #10035: [FLINK-14080][table-planner-blink] Introduce Timestamp
as internal representation of TIMESTAMP_WITHOUT_TIME_ZONE
URL: https://github.com/apache/flink/pull/10035#issuecomment-548206092
 
 
   > We always need the milli part in computing. 
   
   For this requirment, I think we should provide a internface of `long getEpochMilliSecond(int
idx)` on `BaseRow` to reduce object construction. Besides of this, I think `Instant` can satisfy
other cases? 
   
   I even though about only adding these methods `long getEpochMilliSecond(int idx)`, `int
getNano(int idx)`, `ZoneId getZoneId(int idx)`.
   
   > Another reason is that instant is specified to LocalZonedTimestampType in the conversion
class. If we use instant to represent TimestampType, it will cause confusion.
   
   I don't think it is a strong reason. The conversion class happens only on the edge of Table
system, it is not an internal representation. For internal representation, I think it's fine
to use `Instant`, because the concetp of `Instant` and this `Timestamp` is totally the same.
And even more, we can use `Instant getEpochInstant(int idx)` to explicitly get the Instant
on UTC timezone. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message