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

Reply via email to