tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1496733912
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3262,11 +3262,21 @@ private static class CastImplementor extends
AbstractRexCallImplementor {
@Override Expression implementSafe(final RexToLixTranslator translator,
final RexCall call, final List<Expression> argValueList) {
- assert call.getOperands().size() == 1;
+ assert call.getOperands().size() <= 2;
final RelDataType sourceType = call.getOperands().get(0).getType();
- // Short-circuit if no cast is required
RexNode arg = call.getOperands().get(0);
+ ConstantExpression formatExpr;
+ if (call.getOperands().size() > 1) {
Review Comment:
Are the only options 1 or 2? in that case `== 2` might be a more informative
check
##########
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)
Review Comment:
I'm noticing a pattern:
We check if format is null, and then we call the right method accordingly.
Is there a general function we can write that takes in 3 parameters (format,
and then the two methods). It would shorten a lot of these switch statements.
let me know why this is or isn't possible
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -318,141 +320,19 @@ private Expression getConvertExpression(
}
case DATE:
- return translateCastToDate(sourceType, operand, defaultExpression);
+ return translateCastToDate(sourceType, operand, format,
defaultExpression);
case TIME:
- return translateCastToTime(sourceType, operand, defaultExpression);
+ return translateCastToTime(sourceType, operand, format,
defaultExpression);
case TIME_WITH_LOCAL_TIME_ZONE:
- switch (sourceType.getSqlTypeName()) {
- case CHAR:
- case VARCHAR:
- 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.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_TIME_WITH_LOCAL_TIME_ZONE
- .method,
- operand));
-
- default:
- return defaultExpression.get();
- }
+ return translateCastToTimeWithLocalTimeZone(sourceType, operand,
defaultExpression);
case TIMESTAMP:
- switch (sourceType.getSqlTypeName()) {
- case CHAR:
- case VARCHAR:
- return Expressions.call(BuiltInMethod.STRING_TO_TIMESTAMP.method,
- 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();
- }
+ return translateCastToTimeStamp(sourceType, operand, format,
defaultExpression);
Review Comment:
should Timestamp be camelcase?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -1345,6 +1345,169 @@ void testCastStringToDateTime(CastType castType,
SqlOperatorFixture f) {
f.checkNull("cast(cast(null as timestamp) as time)");
}
+ @Test void testCastFormatClauseDateTimeToString() {
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlStdOperatorTable.CAST);
+
+ // Cast DATE to String
+ f.checkString("cast(date '2018-01-30' as varchar format 'YYYY')",
+ "2018",
+ "VARCHAR NOT NULL");
+
+ if (Bug.CALCITE_6269_FIXED) {
+ f.checkString("cast(date '12018-01-30' as varchar format 'YYYY')",
+ "12018",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'Y')",
+ "8",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'YYY')",
+ "018",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'MONTH')",
+ "JANUARY",
+ "VARCHAR NOT NULL");
+ }
+
+ f.checkString("cast(date '2018-01-30' as varchar format 'MON')",
+ "Jan",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'MM')",
+ "01",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'DAY')",
+ "Tuesday",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'DY')",
+ "Tue",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'D')",
+ "3",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'DD')",
+ "30",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-06-30' as varchar format 'DDD')",
+ "181",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2018-01-30' as varchar format 'MM-DD-YY')",
+ "01-30-18",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(date '2021-12-21' as varchar format 'YY Q MON DD')",
+ "21 4 Dec 21",
+ "VARCHAR NOT NULL");
+
+ // Cast TIME to String
+ f.checkString("cast(time '21:30:00' as varchar format 'HH12')",
+ "09",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '1:30:00' as varchar format 'HH24')",
+ "01",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '11:24:00' as varchar format 'MI')",
+ "24",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '21:30:25.16' as varchar format 'SS')",
+ "25",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '15:45:10' as varchar format 'HH12:MI')",
+ "03:45",
+ "VARCHAR NOT NULL");
+
+ if (Bug.CALCITE_6269_FIXED) {
+ f.checkString("cast(time '21:30:25.16' as varchar format 'SSSSS')",
+ "25",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '23:30:55.43' as varchar format 'FF1')",
+ "4",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '23:30:55.43' as varchar format 'AM')",
+ "PM",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(time '12:30:55' as varchar format 'PM')",
+ "PM",
+ "VARCHAR NOT NULL");
+ }
+
+ // Cast TIMESTAMP to String
+ if (Bug.CALCITE_6269_FIXED) {
+ // Query output cannot be validated as it's dependent on execution time
zone
+ f.checkQuery("cast(timestamp '2008-12-25 00:00:00+06:00' as varchar
format 'TZH')");
+ f.checkString("cast(timestamp '2008-12-25 00:00:00+00:00' as varchar
format "
+ + "'TZM' AT TIME ZONE 'Asia/Kolkata')",
+ "30",
+ "VARCHAR NOT NULL");
+ }
+ }
+
+ @Test void testCastFormatClauseStringToDateTime() {
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlStdOperatorTable.CAST);
+ f.checkScalar("cast('18-12-03' as date format 'YY-MM-DD')",
+ "2018-12-03",
+ "DATE NOT NULL");
+ f.checkScalar("cast('JUN 30, 2018' as date format 'MON DD, YYYY')",
+ "2018-06-30",
+ "DATE NOT NULL");
+ f.checkScalar("cast('17:30' as time format 'HH12:MI')",
+ "17:30:00",
+ "TIME(0) NOT NULL");
+ f.checkScalar("cast('01:05:07.16' as time format 'HH24:MI:SS.FF4')",
+ "01:05:07",
+ "TIME(0) NOT NULL");
+ f.checkScalar("cast('2017-05-12' as timestamp format 'YYYY-MM-DD')",
+ "2017-05-12 00:00:00",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("cast('2020.06.03 12:42:53' as timestamp format 'YYYY.MM.DD
HH:MI:SS')",
+ "2020-06-03 12:42:53",
+ "TIMESTAMP(0) NOT NULL");
+
+ if (Bug.CALCITE_6269_FIXED) {
+ f.checkScalar("cast('2020.06.03 00:00:53+06:30' as timestamp format"
+ + " 'YYYY.MM.DD HH24:MI:SSTZH:TZM')",
+ "2020-06-02 17:30:53 UTC",
+ "TIMESTAMP(0) NOT NULL");
+ f.checkScalar("cast('03:30 P.M.' as time format 'HH:MI P.M.')",
+ "15:30:00",
+ "TIME(0) NOT NULL");
+ }
+ }
+
+ @Test void testCastFormatClauseByteToString() {
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlStdOperatorTable.CAST);
+
+ if (Bug.CALCITE_6270_FIXED) {
+ f.checkString("cast(b'\\x48\\x65\\x6c\\x6c\\x6f' as varchar format
'ASCII')",
+ "Hello",
+ "VARCHAR");
+ f.checkScalar("cast('Hello' as varbinary format 'ASCII')",
+ "\\x48\\x65\\x6c\\x6c\\x6f",
+ "VARBINARY NOT NULL");
+ }
+ }
+
+ @Test void testCastFormatClauseNumericToString() {
+ final SqlOperatorFixture f = fixture()
+ .withLibrary(SqlLibrary.BIG_QUERY)
+ .setFor(SqlStdOperatorTable.CAST);
+
+ if (Bug.CALCITE_6270_FIXED) {
+ f.checkString("cast(-12.23 as varchar FORMAT '999.999')",
+ "-12.230",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(1234.56 as varchar FORMAT '$999,999.999')",
+ "$1,234.560",
+ "VARCHAR NOT NULL");
+ f.checkString("cast(123456 as varchar FORMAT '9.999EEEE')",
+ "1.235E+05",
Review Comment:
Does adding a test for `SAFE_CAST` give us any value here?
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2267,16 +2267,21 @@ && sameTypeOrNarrowsNullability(e.getType(),
intExpr.getType())) {
break;
}
final List<RexNode> reducedValues = new ArrayList<>();
- final RexNode simplifiedExpr =
- rexBuilder.makeCast(e.getType(), operand, safe, safe);
+ final RexNode simplifiedExpr = e.operands.size() > 1
+ ? rexBuilder.makeCast(e.getType(), operand, safe, safe,
+ (RexLiteral) e.getOperands().get(1))
+ : rexBuilder.makeCast(e.getType(), operand, safe, safe);
executor.reduce(rexBuilder, ImmutableList.of(simplifiedExpr),
reducedValues);
return requireNonNull(
Iterables.getOnlyElement(reducedValues));
default:
if (operand == e.getOperands().get(0)) {
return e;
} else {
- return rexBuilder.makeCast(e.getType(), operand, safe, safe);
+ return e.operands.size() > 1
Review Comment:
Or can this be 2 or 3? I've confused myself
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -832,6 +858,25 @@ public RexNode makeAbstractCast(RelDataType type, RexNode
exp, boolean safe) {
return new RexCall(type, operator, ImmutableList.of(exp));
}
+ /**
+ * Creates a call to CAST or SAFE_CAST operator with a FORMAT clause.
+ *
+ * @param type Type to cast to
+ * @param exp Expression being cast
+ * @param safe Whether to return NULL if cast fails
+ * @param format Conversion format for target type
+ * @return Call to CAST operator
+ */
+ public RexNode makeAbstractCast(RelDataType type, RexNode exp, boolean safe,
RexLiteral format) {
+ SqlOperator operator =
Review Comment:
operator can be final?
##########
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()),
Review Comment:
The declaring class is always SqlFunctions right? Or no? If it's always the
same we can just pass the class directly instead of calling this getter? Same
comment for above
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3262,11 +3262,21 @@ private static class CastImplementor extends
AbstractRexCallImplementor {
@Override Expression implementSafe(final RexToLixTranslator translator,
final RexCall call, final List<Expression> argValueList) {
- assert call.getOperands().size() == 1;
+ assert call.getOperands().size() <= 2;
final RelDataType sourceType = call.getOperands().get(0).getType();
- // Short-circuit if no cast is required
RexNode arg = call.getOperands().get(0);
+ ConstantExpression formatExpr;
+ if (call.getOperands().size() > 1) {
Review Comment:
nit: can we add `getOperandCount()` to `RexCall`? It's supported by other
similar classes
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2267,16 +2267,21 @@ && sameTypeOrNarrowsNullability(e.getType(),
intExpr.getType())) {
break;
}
final List<RexNode> reducedValues = new ArrayList<>();
- final RexNode simplifiedExpr =
- rexBuilder.makeCast(e.getType(), operand, safe, safe);
+ final RexNode simplifiedExpr = e.operands.size() > 1
+ ? rexBuilder.makeCast(e.getType(), operand, safe, safe,
+ (RexLiteral) e.getOperands().get(1))
+ : rexBuilder.makeCast(e.getType(), operand, safe, safe);
executor.reduce(rexBuilder, ImmutableList.of(simplifiedExpr),
reducedValues);
return requireNonNull(
Iterables.getOnlyElement(reducedValues));
default:
if (operand == e.getOperands().get(0)) {
return e;
} else {
- return rexBuilder.makeCast(e.getType(), operand, safe, safe);
+ return e.operands.size() > 1
Review Comment:
Same comment as above about `== 2`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
