You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "Anthrino (via GitHub)" <gi...@apache.org> on 2024/02/08 21:54:47 UTC
[PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Anthrino opened a new pull request, #3677:
URL: https://github.com/apache/calcite/pull/3677
(no comment)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-1960624156
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [88.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498162592
##########
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:
Yeah that makes sense. I think explicitly passing the class is more readable, it tells the user exactly where the method is defined. I also think it conforms better with the rest of Calcite. Up to you though
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1483662654
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -684,7 +686,13 @@ protected RexNode convertCast(
final boolean safe = kind == SqlKind.SAFE_CAST;
final SqlNode left = call.operand(0);
final SqlNode right = call.operand(1);
+ final SqlNode format = call.getOperandList().size() > 2 ?
+ call.operand(2) : null;
+
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final RexNode arg = cx.convertExpression(left);
+ final RexNode formatArg = isNull(format) ? null : cx.convertExpression(format);
Review Comment:
Why use the `isNull` method, here and elsewhere? `isNull` detects the `NULL` literal, but I don't think that `CAST(d AS DATE FORMAT NULL)` is even allowed (correct me if I'm wrong).
The only null is a java null, where you pass a RexLiteral whose value is null if there was no `FORMAT` clause.
I am wondering whether you intend to support an expression for the format clause, e.g. `CAST(d AS DATE FORMAT 'yyyy-' || 'mm')` but as there are no tests I can't answer that question.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1522185271
##########
core/src/main/java/org/apache/calcite/util/format/FormatModels.java:
##########
@@ -142,9 +142,7 @@ MI, literalElement(":"), SS, literalElement(" "),
map.put("%Y", YYYY);
map.put("%y", YY);
map.put("%Z", TZR);
- BIG_QUERY = create(map);
- map.clear();
Review Comment:
That was clearing off all the previous records added to the `FormatElements` map after creating the `BigQuery` FormatModel (line 145), to add new values and create the `POSTGRESQL` model (line 177).
All the below format elements used by `POSTGRESQL` are also supposed to be available in `BigQuery`, so instead of clearing it we can add all those elements and create both models at the end.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498357623
##########
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:
Ok no need to make this PR any bigger if it doesn't give us anything so it's up to you
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498350724
##########
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:
Yeah either or
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1501127168
##########
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:
A few other SQL functions are implemented by non-static methods (generally to allow caching of expensive state). Do whatever those functions do.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1556287356
##########
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:
Good to have the working tests enabled, and the disabled ones gives a status on whats yet to be supported and need to be addressed in followup JIRAs.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1554338431
##########
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:
I think that override is required in every function implementation to specify the number of operands being passed, it was already existing with a count = 2 just updated that to be 2 or 3.
Removing that does fail a number of tests and breaks the build, it throws the `Util.needToImplement` exception from the super as `operandTypeChecker` is not defined and we need an override.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498176623
##########
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:
SGTM 👍
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498141654
##########
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:
Makes sense, yes the options should only be 1 or 2 for now, if we have a `AT <Timezone>` clause added in later it would become 3, but irrespective we should make it a deterministic equality check.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498349778
##########
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:
`SqlCall` has `<S extends SqlNode> S operand(int)` and `int operandCount()`. I think we should follow that model.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1554345497
##########
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:
Not exactly, we could only move the `RexImpTable.optimize2()` call outside the switch, but the specific method called to evaluate the CAST depends on the from and to cast data types, and with this change also if there is a `FORMAT` present. So the number of lines would still look the same could move out one level of nesting with that optimize call and return statements outside. Does that seem like a better pattern?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1556267957
##########
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:
Looks like it, didnt get any exceptions with the style check maybe github did a weird line wrap
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1523651994
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -826,12 +852,31 @@ public RexNode makeAbstractCast(RelDataType type, RexNode exp) {
* @return Call to CAST operator
*/
public RexNode makeAbstractCast(RelDataType type, RexNode exp, boolean safe) {
- SqlOperator operator =
+ final SqlOperator operator =
safe ? SqlLibraryOperators.SAFE_CAST
: SqlStdOperatorTable.CAST;
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) {
Review Comment:
oh duh
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-2050706514
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [90.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498350848
##########
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:
In situations like this, I use `> 1`. It is more future-proof, and asks for only what it needs.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1500130352
##########
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:
Changed all the tests to use `safeParameters()` to test all CAST functions.
##########
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:
Changed all the tests to use `safeParameters()` pattern to test all CAST functions.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-2038347616
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [14 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [90.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1554037990
##########
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:
That's right @tanclary, we considered that and I think Julian suggested we go with this method name to follow the `SqlCall` pattern:
`SqlCall` has `<S extends SqlNode> S operand(int)` and `int operandCount()`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-1955698412
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [89.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1523648743
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -826,12 +852,31 @@ public RexNode makeAbstractCast(RelDataType type, RexNode exp) {
* @return Call to CAST operator
*/
public RexNode makeAbstractCast(RelDataType type, RexNode exp, boolean safe) {
- SqlOperator operator =
+ final SqlOperator operator =
safe ? SqlLibraryOperators.SAFE_CAST
: SqlStdOperatorTable.CAST;
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) {
Review Comment:
If you're referring to the format argument being null, we're creating a null valued RexLiteral object if we receive an empty format string from the parser, so it doesnt need to be nullable.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1522177576
##########
core/src/main/java/org/apache/calcite/util/format/FormatModels.java:
##########
@@ -142,9 +142,7 @@ MI, literalElement(":"), SS, literalElement(" "),
map.put("%Y", YYYY);
map.put("%y", YY);
map.put("%Z", TZR);
- BIG_QUERY = create(map);
- map.clear();
Review Comment:
why get rid of this
##########
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##########
@@ -6218,6 +6218,22 @@ private static Matcher<SqlNode> isCharLiteral(String s) {
.ok("CAST(`A` AS MAP< VARCHAR MULTISET, MAP< INTEGER, INTEGER > >)");
}
+ @Test void testFormatClauseInCast() {
Review Comment:
while adding comments may as well add one here to case
##########
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.operandCount() <= 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.operandCount() == 2) {
Review Comment:
comment here could be good
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -826,12 +852,31 @@ public RexNode makeAbstractCast(RelDataType type, RexNode exp) {
* @return Call to CAST operator
*/
public RexNode makeAbstractCast(RelDataType type, RexNode exp, boolean safe) {
- SqlOperator operator =
+ final SqlOperator operator =
safe ? SqlLibraryOperators.SAFE_CAST
: SqlStdOperatorTable.CAST;
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) {
Review Comment:
does this need to be nullable
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -239,6 +245,12 @@ private static RelDataType createTypeWithNullabilityFromExpr(RelDataTypeFactory
}
return false;
}
+
+ // Validate format argument is string type if included
Review Comment:
can this be `return <insert conditions>`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498178354
##########
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:
Yes that should work, will make the change to the existing `makeAbstractCast` overload too.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498181295
##########
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:
It can be upto 2 with this implementation, will change to equality check.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1554345497
##########
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:
Not exactly, we could only move the `RexImpTable.optimize2()` call outside the switch, but the specific method called to evaluate the CAST depends on the from and to cast data types, and with this change also if there is a `FORMAT` present. So the number of lines would still look the same, but we could move out one level of nesting with the optimize calls and return statements outside the switch case.
Does that seem like a better pattern?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-1949411750
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [12 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [78.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1496704666
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -684,7 +686,13 @@ protected RexNode convertCast(
final boolean safe = kind == SqlKind.SAFE_CAST;
final SqlNode left = call.operand(0);
final SqlNode right = call.operand(1);
+ final SqlNode format = call.getOperandList().size() > 2 ?
+ call.operand(2) : null;
+
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final RexNode arg = cx.convertExpression(left);
+ final RexNode formatArg = isNull(format) ? null : cx.convertExpression(format);
Review Comment:
Thanks for checking this Julian, I intended to use a Null-valued RexLiteral in cases where the format clause is not specified with the CAST expression. I've updated the flow to populate/check the constant null literal where required.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1523650335
##########
core/src/main/java/org/apache/calcite/util/format/FormatModels.java:
##########
@@ -142,9 +142,7 @@ MI, literalElement(":"), SS, literalElement(" "),
map.put("%Y", YYYY);
map.put("%y", YY);
map.put("%Z", TZR);
- BIG_QUERY = create(map);
- map.clear();
Review Comment:
There would be some restructuring / additions to this set of FormatElements based on the changes required in [CALCITE-6269](https://issues.apache.org/jira/browse/CALCITE-6269)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1554345497
##########
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:
Not exactly, we could actually only move the `RexImpTable.optimize2()` call outside the switch, but the specific method called to evaluate the CAST depends on the from and to cast data types, and with this change also if there is a `FORMAT` present. So the number of lines would still look the same could move out one level of nesting with that optimize call and return statements outside. Does that seem like a better pattern?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary merged PR #3677:
URL: https://github.com/apache/calcite/pull/3677
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498354415
##########
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:
In the future if there was another argument we may not want it to follow the same logic, that's why I think equivalency is a better check.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498158155
##########
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 functions are from `SqlFunctions` but might belong to subclasses, e.g.
```
FORMAT_DATE => SqlFunctions.DateFormatFunction.class
PARSE_TIME => SqlFunctions.DateParseFunction.class
```
my assumption is we need to pass the inner class name as the `targetExpression`, I am not completely aware of the use of this [`targetExpression`](https://github.com/apache/calcite/blob/7ca6fc4bdd3e1274a62f00f4c8d5830162a74879/linq4j/src/main/java/org/apache/calcite/linq4j/tree/MethodCallExpression.java#L33) attribute in the `MethodCallExpression` class, if anyone else can confirm?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1501265054
##########
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:
Thanks for the pointers, I couldn't find an explicit usage of a non-static built-in method in `Expressions.call()`, but saw `getDeclaringClass()` used to build the target expression so using that pattern; I think it's easier to trace to the class of the method call even though its a slight hit on readability.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3677:
URL: https://github.com/apache/calcite/pull/3677#issuecomment-1939709400
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677) **Quality Gate passed**
Issues
 [9 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3677&resolved=false&inNewCodePeriod=true)
 [78.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3677&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3677)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498174614
##########
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:
That's right, the reason for the many switch cases are because we need to evaluate the expressions based on each combination of source and target data types (e.g. `CAST VARCHAR to TIME/DATE/NUMERIC..`), and each combination has an individual `BuiltInMethod` implementation so the calls have to made explicit.
We could look at moving the method selection (based on `format` condition) out of the returned method call, but it'll be the same number of lines to assign the respective method under each case, so I dont think it would reduce the clutter by much. Lack of generalizability is due to the number of cases and individual built in methods for each case.
##########
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:
That's right, the reason for the many switch cases are because we need to evaluate the expressions based on each combination of source and target data types (e.g. `CAST VARCHAR to TIME/DATE/NUMERIC..`), and each combination has an individual `BuiltInMethod` implementation so the calls have to made explicit.
We could look at moving the method selection (based on `format` condition) out of the returned method call, but it'll be the same number of lines to assign the respective method under each case, so I dont think it would reduce the clutter by much. Lack of generalizability is due to the number of cases and individual built in methods for each 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.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498182416
##########
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:
Essentially both operators should take the same flow, I will add some tests to see it there's any deviation in functionality.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1498354415
##########
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:
In the future if there was another argument we may not want it to follow the same logic, that's why I think equivalency is a better check. It feels presumptuous to assume any operand count >1 should follow this.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1556274714
##########
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:
I don't think its an added newline, just the line separators between each case statement like it was there before?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "Anthrino (via GitHub)" <gi...@apache.org>.
Anthrino commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1523640558
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -239,6 +245,12 @@ private static RelDataType createTypeWithNullabilityFromExpr(RelDataTypeFactory
}
return false;
}
+
+ // Validate format argument is string type if included
Review Comment:
Good catch, simplified into single return statement!
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-2980] Implement the FORMAT clause of the CAST operator [calcite]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3677:
URL: https://github.com/apache/calcite/pull/3677#discussion_r1523645556
##########
core/src/main/java/org/apache/calcite/util/format/FormatModels.java:
##########
@@ -142,9 +142,7 @@ MI, literalElement(":"), SS, literalElement(" "),
map.put("%Y", YYYY);
map.put("%y", YY);
map.put("%Z", TZR);
- BIG_QUERY = create(map);
- map.clear();
Review Comment:
Nice!
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org