tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1553481833


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -647,58 +541,225 @@ private static Expression checkExpressionPadTruncate(
     }
   }
 
+  private Expression translateCastToDate(RelDataType sourceType,
+      Expression operand, ConstantExpression format,
+      Supplier<Expression> defaultExpression) {
+
+    switch (sourceType.getSqlTypeName()) {
+    case CHAR:
+    case VARCHAR:
+      // If format string is supplied, parse formatted string into date
+      return Expressions.isConstantNull(format)
+          ? Expressions.call(BuiltInMethod.STRING_TO_DATE.method, operand)
+          : 
Expressions.call(Expressions.new_(BuiltInMethod.PARSE_DATE.method.getDeclaringClass()),
+              BuiltInMethod.PARSE_DATE.method, format, operand);
+
+    case TIMESTAMP:
+      return
+          Expressions.convert_(
+              Expressions.call(BuiltInMethod.FLOOR_DIV.method,
+                  operand, Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
+              int.class);
+
+    case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_DATE.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+
+    default:
+      return defaultExpression.get();
+    }
+  }
+
   private Expression translateCastToTime(RelDataType sourceType,
-      Expression operand, Supplier<Expression> defaultExpression) {
+      Expression operand, ConstantExpression format, Supplier<Expression> 
defaultExpression) {
+
     switch (sourceType.getSqlTypeName()) {
     case CHAR:
     case VARCHAR:
-      return Expressions.call(BuiltInMethod.STRING_TO_TIME.method, operand);
+      // If format string is supplied, parse formatted string into time
+      return Expressions.isConstantNull(format)
+          ? Expressions.call(BuiltInMethod.STRING_TO_TIME.method, operand)
+          : 
Expressions.call(Expressions.new_(BuiltInMethod.PARSE_TIME.method.getDeclaringClass()),
+              BuiltInMethod.PARSE_TIME.method, format, operand);
 
     case TIME_WITH_LOCAL_TIME_ZONE:
-      return RexImpTable.optimize2(operand,
-          Expressions.call(
-              BuiltInMethod.TIME_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
-              operand,
-              Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIME_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
 
     case TIMESTAMP:
-      return Expressions.convert_(
-          Expressions.call(BuiltInMethod.FLOOR_MOD.method,
-              operand,
-              Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
-          int.class);
+      return
+          Expressions.convert_(
+              Expressions.call(BuiltInMethod.FLOOR_MOD.method,
+                  operand,
+                  Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
+              int.class);
+
 
     case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
-      return RexImpTable.optimize2(operand,
-          Expressions.call(
-              BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
-              operand,
-              Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
 
     default:
       return defaultExpression.get();
     }
   }
 
-  private Expression translateCastToDate(RelDataType sourceType,
+  private Expression translateCastToTimeWithLocalTimeZone(RelDataType 
sourceType,
       Expression operand, Supplier<Expression> defaultExpression) {
+
     switch (sourceType.getSqlTypeName()) {
     case CHAR:
     case VARCHAR:
-      return Expressions.call(BuiltInMethod.STRING_TO_DATE.method, operand);
+      return
+          
Expressions.call(BuiltInMethod.STRING_TO_TIME_WITH_LOCAL_TIME_ZONE.method, 
operand);
+
+    case TIME:
+      return
+          
Expressions.call(BuiltInMethod.TIME_STRING_TO_TIME_WITH_LOCAL_TIME_ZONE.method,
+              RexImpTable.optimize2(operand,
+                  Expressions.call(BuiltInMethod.UNIX_TIME_TO_STRING.method,
+                      operand)),
+              Expressions.call(BuiltInMethod.TIME_ZONE.method, root));
 
     case TIMESTAMP:
-      return Expressions.convert_(
-          Expressions.call(BuiltInMethod.FLOOR_DIV.method,
-              operand, Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
-          int.class);
+      return
+          
Expressions.call(BuiltInMethod.TIMESTAMP_STRING_TO_TIMESTAMP_WITH_LOCAL_TIME_ZONE.method,
+              RexImpTable.optimize2(operand,
+                  
Expressions.call(BuiltInMethod.UNIX_TIMESTAMP_TO_STRING.method,
+                      operand)),
+              Expressions.call(BuiltInMethod.TIME_ZONE.method, root));
 
     case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
-      return RexImpTable.optimize2(operand,
-          Expressions.call(
-              BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_DATE.method,
-              operand,
-              Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod
+                      
.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIME_WITH_LOCAL_TIME_ZONE
+                      .method,
+                  operand));
+
+    default:
+      return defaultExpression.get();
+    }
+  }
+
+  private Expression translateCastToTimestamp(RelDataType sourceType,
+      Expression operand, ConstantExpression format, Supplier<Expression> 
defaultExpression) {
+
+    switch (sourceType.getSqlTypeName()) {
+    case CHAR:
+    case VARCHAR:
+      // If format string is supplied, parse formatted string into timestamp
+      return Expressions.isConstantNull(format)
+          ? Expressions.call(BuiltInMethod.STRING_TO_TIMESTAMP.method, operand)
+          : Expressions.call(
+              
Expressions.new_(BuiltInMethod.PARSE_TIMESTAMP.method.getDeclaringClass()),
+              BuiltInMethod.PARSE_TIMESTAMP.method, format, operand);
+
+    case DATE:
+      return
+          Expressions.multiply(Expressions.convert_(operand, long.class),
+              Expressions.constant(DateTimeUtils.MILLIS_PER_DAY));
+
+    case TIME:
+      return
+          Expressions.add(
+              Expressions.multiply(
+                  Expressions.convert_(
+                      Expressions.call(BuiltInMethod.CURRENT_DATE.method, 
root),
+                      long.class),
+                  Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
+              Expressions.convert_(operand, long.class));
+
+    case TIME_WITH_LOCAL_TIME_ZONE:
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIME_WITH_LOCAL_TIME_ZONE_TO_TIMESTAMP.method,
+                  Expressions.call(BuiltInMethod.UNIX_DATE_TO_STRING.method,
+                      Expressions.call(BuiltInMethod.CURRENT_DATE.method, 
root)),
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+
+    case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  
BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIMESTAMP.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+
+    default:
+      return defaultExpression.get();
+    }
+  }
+
+  private Expression translateCastToTimestampWithLocalTimeZone(RelDataType 
sourceType,
+      Expression operand, Supplier<Expression> defaultExpression) {
+
+    switch (sourceType.getSqlTypeName()) {
+    case CHAR:
+    case VARCHAR:
+      return
+          
Expressions.call(BuiltInMethod.STRING_TO_TIMESTAMP_WITH_LOCAL_TIME_ZONE.method,
+              operand);
+

Review Comment:
   do we want newline here



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -647,58 +541,225 @@ private static Expression checkExpressionPadTruncate(
     }
   }
 
+  private Expression translateCastToDate(RelDataType sourceType,
+      Expression operand, ConstantExpression format,
+      Supplier<Expression> defaultExpression) {
+
+    switch (sourceType.getSqlTypeName()) {
+    case CHAR:
+    case VARCHAR:
+      // If format string is supplied, parse formatted string into date
+      return Expressions.isConstantNull(format)
+          ? Expressions.call(BuiltInMethod.STRING_TO_DATE.method, operand)
+          : 
Expressions.call(Expressions.new_(BuiltInMethod.PARSE_DATE.method.getDeclaringClass()),
+              BuiltInMethod.PARSE_DATE.method, format, operand);
+
+    case TIMESTAMP:
+      return
+          Expressions.convert_(
+              Expressions.call(BuiltInMethod.FLOOR_DIV.method,
+                  operand, Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
+              int.class);
+
+    case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_DATE.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+
+    default:
+      return defaultExpression.get();
+    }
+  }
+
   private Expression translateCastToTime(RelDataType sourceType,
-      Expression operand, Supplier<Expression> defaultExpression) {
+      Expression operand, ConstantExpression format, Supplier<Expression> 
defaultExpression) {
+
     switch (sourceType.getSqlTypeName()) {
     case CHAR:
     case VARCHAR:
-      return Expressions.call(BuiltInMethod.STRING_TO_TIME.method, operand);
+      // If format string is supplied, parse formatted string into time
+      return Expressions.isConstantNull(format)
+          ? Expressions.call(BuiltInMethod.STRING_TO_TIME.method, operand)
+          : 
Expressions.call(Expressions.new_(BuiltInMethod.PARSE_TIME.method.getDeclaringClass()),
+              BuiltInMethod.PARSE_TIME.method, format, operand);
 
     case TIME_WITH_LOCAL_TIME_ZONE:
-      return RexImpTable.optimize2(operand,
-          Expressions.call(
-              BuiltInMethod.TIME_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
-              operand,
-              Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIME_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
 
     case TIMESTAMP:
-      return Expressions.convert_(
-          Expressions.call(BuiltInMethod.FLOOR_MOD.method,
-              operand,
-              Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
-          int.class);
+      return
+          Expressions.convert_(
+              Expressions.call(BuiltInMethod.FLOOR_MOD.method,
+                  operand,
+                  Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
+              int.class);
+
 
     case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
-      return RexImpTable.optimize2(operand,
-          Expressions.call(
-              BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
-              operand,
-              Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
+      return
+          RexImpTable.optimize2(
+              operand, Expressions.call(
+                  BuiltInMethod.TIMESTAMP_WITH_LOCAL_TIME_ZONE_TO_TIME.method,
+                  operand,
+                  Expressions.call(BuiltInMethod.TIME_ZONE.method, root)));
 
     default:
       return defaultExpression.get();
     }
   }
 
-  private Expression translateCastToDate(RelDataType sourceType,
+  private Expression translateCastToTimeWithLocalTimeZone(RelDataType 
sourceType,
       Expression operand, Supplier<Expression> defaultExpression) {
+
     switch (sourceType.getSqlTypeName()) {
     case CHAR:
     case VARCHAR:
-      return Expressions.call(BuiltInMethod.STRING_TO_DATE.method, operand);
+      return
+          
Expressions.call(BuiltInMethod.STRING_TO_TIME_WITH_LOCAL_TIME_ZONE.method, 
operand);
+
+    case TIME:
+      return
+          
Expressions.call(BuiltInMethod.TIME_STRING_TO_TIME_WITH_LOCAL_TIME_ZONE.method,
+              RexImpTable.optimize2(operand,
+                  Expressions.call(BuiltInMethod.UNIX_TIME_TO_STRING.method,
+                      operand)),
+              Expressions.call(BuiltInMethod.TIME_ZONE.method, root));
 
     case TIMESTAMP:
-      return Expressions.convert_(
-          Expressions.call(BuiltInMethod.FLOOR_DIV.method,
-              operand, Expressions.constant(DateTimeUtils.MILLIS_PER_DAY)),
-          int.class);
+      return
+          
Expressions.call(BuiltInMethod.TIMESTAMP_STRING_TO_TIMESTAMP_WITH_LOCAL_TIME_ZONE.method,

Review Comment:
   is this aligned right? displaying weird on github



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -469,34 +349,48 @@ private Expression getConvertExpression(
       final SqlIntervalQualifier interval =
           sourceType.getIntervalQualifier();
       switch (sourceType.getSqlTypeName()) {
+      // If format string is supplied, return formatted date/time/timestamp
       case DATE:
-        return RexImpTable.optimize2(operand,
-            Expressions.call(BuiltInMethod.UNIX_DATE_TO_STRING.method,
-                operand));
+        return RexImpTable.optimize2(operand, 
Expressions.isConstantNull(format)
+            ? Expressions.call(BuiltInMethod.UNIX_DATE_TO_STRING.method, 
operand)
+            : Expressions.call(
+                Expressions.new_(
+                    BuiltInMethod.FORMAT_DATE.method.getDeclaringClass()),
+                BuiltInMethod.FORMAT_DATE.method, format, operand));
 
       case TIME:
-        return RexImpTable.optimize2(operand,
-            Expressions.call(BuiltInMethod.UNIX_TIME_TO_STRING.method,
-                operand));
+        return RexImpTable.optimize2(operand, 
Expressions.isConstantNull(format)

Review Comment:
   can you evaluate this before the `switch` ? then you can just check the 
variable in each case



##########
core/src/main/java/org/apache/calcite/rex/RexCall.java:
##########
@@ -251,6 +251,10 @@ public List<RexNode> getOperands() {
     return operands;
   }
 
+  public int operandCount() {

Review Comment:
   i might have been the one to suggest this, but is `getOperandCount` better 
since it follows typical getter naming convention



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -191,12 +194,12 @@ private static RelDataType 
createTypeWithNullabilityFromExpr(RelDataTypeFactory
   }
 
   @Override public String getSignatureTemplate(final int operandsCount) {
-    assert operandsCount == 2;
-    return "{0}({1} AS {2})";
+    assert operandsCount <= 3;
+    return "{0}({1} AS {2} [FORMAT {3}])";
   }
 
   @Override public SqlOperandCountRange getOperandCountRange() {
-    return SqlOperandCountRanges.of(2);
+    return SqlOperandCountRanges.between(2, 3);

Review Comment:
   does this override still need to exist ? maybe try deleting it and see what 
happens. the super looks like it should do what you need



##########
core/src/test/resources/sql/cast-with-format.iq:
##########
@@ -517,173 +530,182 @@ select cast('2010 14 january' as date FORMAT
 EXPR$0
 2010-01-14
 !ok
+!}
 
+!if (false) {
+### disabled until Bug.CALCITE_6269_FIXED ###
 # Test different lowercase vs uppercase scenarios with the datetime to string 
path.
-select cast(date'2010-10-18' as string FORMAT
+select cast(date'2010-10-18' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 OCTOBER   October   october
 !ok
 
-select cast(cast('2010-11-18' as timestamp) as string
+select cast(cast('2010-11-18' as timestamp) as varchar
     FORMAT 'MONTH Month month');
 EXPR$0
 NOVEMBER  November  november
 !ok
 
-select cast(date'2010-12-19' as string FORMAT
+select cast(date'2010-12-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 DECEMBER  December  december
 !ok
 
-select cast(date'2010-01-19' as string FORMAT
+select cast(date'2010-01-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 JANUARY   January   january
 !ok
 
-select cast(date'2010-02-19' as string FORMAT
+select cast(date'2010-02-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 FEBRUARY  February  february
 !ok
 
-select cast(date'2010-03-19' as string FORMAT
+select cast(date'2010-03-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 MARCH     March     march
 !ok
 
-select cast(date'2010-04-19' as string FORMAT
+select cast(date'2010-04-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 APRIL     April     april
 !ok
 
-select cast(date'2010-05-19' as string FORMAT
+select cast(date'2010-05-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 MAY       May       may
 !ok
 
-select cast(date'2010-06-19' as string FORMAT
+select cast(date'2010-06-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 JUNE      June      june
 !ok
 
-select cast(date'2010-07-19' as string FORMAT
+select cast(date'2010-07-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 JULY      July      july
 !ok
 
-select cast(date'2010-08-19' as string FORMAT
+select cast(date'2010-08-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 AUGUST    August    august
 !ok
 
-select cast(date'2010-09-19' as string FORMAT
+select cast(date'2010-09-19' as varchar FORMAT
     'MONTH Month month');
 EXPR$0
 SEPTEMBER September september
 !ok
+!}
 
+!if (false) {
+### disabled until Bug.CALCITE_6269_FIXED ###
 # Test odd casing of month token.
-select cast(date'2010-09-20' as string FORMAT
+select cast(date'2010-09-20' as varchar FORMAT
     'MOnth MONth MONTh');
 EXPR$0
 SEPTEMBER SEPTEMBER SEPTEMBER
 !ok
 
-select cast(date'2010-09-21' as string FORMAT
+select cast(date'2010-09-21' as varchar FORMAT
     'montH monTH moNTH moNTH');
 EXPR$0
 september september september september
 !ok
 
 # Test different lowercase vs uppercase scenarios with the datetime to string 
path
 # when FM is provided.
-select cast(date'2010-10-18' as string FORMAT
+select cast(date'2010-10-18' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 OCTOBER October october
 !ok
 
-select cast(cast('2010-11-18' as timestamp) as string
+select cast(cast('2010-11-18' as timestamp) as varchar
     FORMAT 'FMMONTH FMMonth FMmonth');
 EXPR$0
 NOVEMBER November november
 !ok
 
-select cast(date'2010-12-19' as string FORMAT
+select cast(date'2010-12-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 DECEMBER December december
 !ok
 
-select cast(date'2010-01-19' as string FORMAT
+select cast(date'2010-01-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 JANUARY January january
 !ok
 
-select cast(date'2010-02-19' as string FORMAT
+select cast(date'2010-02-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 FEBRUARY February february
 !ok
 
-select cast(date'2010-03-19' as string FORMAT
+select cast(date'2010-03-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 MARCH March march
 !ok
 
-select cast(date'2010-04-19' as string FORMAT
+select cast(date'2010-04-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 APRIL April april
 !ok
 
-select cast(date'2010-05-19' as string FORMAT
+select cast(date'2010-05-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 MAY May may
 !ok
 
-select cast(date'2010-06-19' as string FORMAT
+select cast(date'2010-06-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 JUNE June june
 !ok
 
-select cast(date'2010-07-19' as string FORMAT
+select cast(date'2010-07-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 JULY July july
 !ok
 
-select cast(date'2010-08-19' as string FORMAT
+select cast(date'2010-08-19' as varchar FORMAT
     'FMMONTH FMMonth FMmonth');
 EXPR$0
 AUGUST August august
 !ok
 
-select cast(date'2010-09-19' as string FORMAT
+select cast(date'2010-09-19' as varchar FORMAT

Review Comment:
   thanks for going through these im sure that took a bit



-- 
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: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to