rdblue commented on code in PR #9008: URL: https://github.com/apache/iceberg/pull/9008#discussion_r1520429456
########## api/src/main/java/org/apache/iceberg/expressions/Literals.java: ########## @@ -298,7 +299,7 @@ public <T> Literal<T> to(Type type) { case TIME: return (Literal<T>) new TimeLiteral(value()); case TIMESTAMP: - return (Literal<T>) new TimestampLiteral(value()); + return (Literal<T>) new TimestampLiteral(((TimestampType) type).unit(), value()); Review Comment: This is a problem because we don't know how to interpret the value. Before, the value was always interpreted as micros, which is probably the right way to update this if we want to preserve behavior: ```java return (Literal<T>) new TimestampLiteral(Unit.MICROS, value()).to(type); ``` This is going to cause another problem, eventually, because v3 is going to introduce type promotion from long to timestamp. The current plan is to always interpret long values as millis because that is the most common JVM representation. But this code uses micros because that's what Spark will pass through. Looks like we may need to update Spark to pass a String instead, although that means converting to/from string which is ugly. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org