rdblue commented on code in PR #9008:
URL: https://github.com/apache/iceberg/pull/9008#discussion_r1494844317
##########
api/src/main/java/org/apache/iceberg/expressions/Literals.java:
##########
@@ -501,18 +531,22 @@ public <T> Literal<T> to(Type type) {
return (Literal<T>) new TimeLiteral(timeMicros);
case TIMESTAMP:
- if (((Types.TimestampType) type).shouldAdjustToUTC()) {
- long timestampMicros =
- ChronoUnit.MICROS.between(
- EPOCH, OffsetDateTime.parse(value(),
DateTimeFormatter.ISO_DATE_TIME));
- return (Literal<T>) new TimestampLiteral(timestampMicros);
+ TimestampType tsType = (TimestampType) type;
+ if (tsType.shouldAdjustToUTC()) {
+ long timestampUnits =
+ tsType
+ .unit()
+ .between(EPOCH, OffsetDateTime.parse(value(),
DateTimeFormatter.ISO_DATE_TIME));
Review Comment:
The conversion logic should be in `DateTimeUtil` rather than embedded here.
There may be methods for doing this already for microseconds. The important
thing is that we want to centralize the logic in `DateTimeUtil` if we are
changing this section. We hadn't done it yet because this code is so old and we
saw no reason to modify it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
