robinyqiu commented on a change in pull request #12054:
URL: https://github.com/apache/beam/pull/12054#discussion_r443906131



##########
File path: 
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java
##########
@@ -315,7 +315,7 @@ private static Expression castOutput(Expression value, 
FieldType toType) {
   private static Expression castOutputTime(Expression value, FieldType toType) 
{
     Expression valueDateTime = value;
 
-    // First, convert to millis (except for DATE type)
+    // First, convert to millis (except for DATE/TIME type)

Review comment:
       Another problem is that `LocalTime.ofNanoOfDay()` takes a `long` instead 
of `Long`. So when the `valueDateTime` is of type `Long` the expression cannot 
be resolved. The fix is to add:
   ```
   if (value.getType() == Long.class) {
     valueDateTime = Expressions.unbox(valueDateTime);
   }
   ```
   (The same fix should probably be added to `DATE` case as well. It has not 
failed simply because no test has hit that case.)




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to