robinyqiu commented on a change in pull request #12054:
URL: https://github.com/apache/beam/pull/12054#discussion_r443904994
##########
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:
All the current failing tests are due to the code below (`TIME` case).
Note the comment says "convert to millis", which means the values are in
milliseconds. Previously it will hit this code path:
`Expressions.new_(Instant.class, valueDateTime);`, which corresponds to calling
`new Instant(long)`, which takes milliseconds as a long as the input parameter.
Now you have updated the code so it will hit calling `LocalTime.ofNanoOfDay()`
which takes nanosecond. The fix will be adding 1 LOC: `valueDateTime =
Expressions.multiply(valueDateTime, Expressions.constant(1000000L))`.
----------------------------------------------------------------
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
