amaliujia commented on a change in pull request #1687: [CALCITE-3612] Add 
TIME/TIMESTAMP WITH TIME ZONE in optimizer
URL: https://github.com/apache/calcite/pull/1687#discussion_r361741594
 
 

 ##########
 File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
 ##########
 @@ -341,6 +344,16 @@ private Expression converterToDate(RelDataType 
sourceType, Expression operand) {
           BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_DATE.method,
           operand,
           Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      break;
+    // SQL 2011 Part2 Section 4.6 General Rules 13.d
+    case TIMESTAMP_WITH_TIME_ZONE:
+      convert = Expressions.convert_(
+        Expressions.call(BuiltInMethod.FLOOR_DIV.method,
 
 Review comment:
   Yes. By reading the SQL standard I got the idea of the conversion path.
   
   This suggestion is more from readability perspective. If you implements one 
more function that then code readers will know the equivalent implementation is 
 `CAST ( CAST ( VE AS TIMESTAMP WITHOUT TIME ZONE ) AS DATE )`.
   
   I had to read SQL standard cause I wasn't sure why there is a "magic" 
operation after casting to `TIMESTAMP WITHOUT TIME ZONE`.
   
   Or maybe you can add a comment instead to say the implementation here is 
`CAST ( CAST ( VE AS TIMESTAMP WITHOUT TIME ZONE ) AS DATE )`

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to