[
https://issues.apache.org/jira/browse/CALCITE-5489?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17681152#comment-17681152
]
Julian Hyde commented on CALCITE-5489:
--------------------------------------
{quote}Sergey's RexBuilderTest seems to validate something that is not
reproducible through the SQL Parser but should probably still be
supported.{quote}
I agree we should support it. And therefore we should keep the RexBuilderTest.
{quote}The failure is due to the opBinding class at the time
SqlTimestampDiffFunction#inferReturnType2 is called. For quidem tests,
opBinding is always type of SqlCallBinding. During RexBuilderTest it is type
RexCallBinding. These have different concepts of what constitutes a "literal"
so the condition opBinding.isOperandLiteral(0, true) is inappropriate
here.{quote}
That makes sense. It had not occurred to me that it was due to different
implementations of {{SqlOperatorBinding}}.
{quote}I'd suggest we make a new entry in SqlKind that is
BIG_QUERY_TIMESTAMP_DIFF. The conditional statements in
SqlTimestampDiffFunction#inferReturnType2 and
TimestampDiffConvertlet#convertCall should branch based on the operator kind.
That way the operator itself has a sense of what it is and we wouldn't have to
check the ordering or types of its operands.{quote}
That's a valid approach. But we try not to add values to {{SqlKind}} for each
and every function, or indeed to create a new class for each function. My
preference these days is to use generic function classes (e.g.
{{SqlBasicFunction}}) and modify the behavior based on the field values of the
function (e.g. the {{SqlOperandTypeChecker}}).
I think for now we should go with Tanner's fix, plus Sergey's test case. We can
refactor later, if any further bugs show up.
I note that BigQuery's {{TIME_DIFF}} has a similar pattern to BigQuery's
{{TIMESTAMP_DIFF}}, namely (datetime, datetime, flag), and would probably be
affected by this bug.
I will push a commit as I described above.
> Cannot convert TIMESTAMP literal to class
> org.apache.calcite.avatica.util.TimeUnit
> ----------------------------------------------------------------------------------
>
> Key: CALCITE-5489
> URL: https://issues.apache.org/jira/browse/CALCITE-5489
> Project: Calcite
> Issue Type: Bug
> Components: core
> Reporter: Sergey Nuyanzin
> Assignee: Tanner Clary
> Priority: Blocker
> Labels: pull-request-available
> Fix For: 1.33.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> It seems it stops working after
> {noformat}
> [CALCITE-5360] Add TIMESTAMP_ADD function (enabled in BigQuery
> library){noformat}
> for {{RexCallBinding}}
> e.g. this test starts failing
> {code:java}
> @Test void testTimestampDiffCall() {
> final RelDataTypeFactory typeFactory = new
> SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
> RexBuilder rexBuilder = new RexBuilder(typeFactory);
> final RexImplicationCheckerFixtures.Fixture f = new
> RexImplicationCheckerFixtures.Fixture();
> final TimestampString ts =
> TimestampString.fromCalendarFields(Util.calendar());
> rexBuilder.makeCall(SqlStdOperatorTable.TIMESTAMP_DIFF,
> ImmutableList.of(rexBuilder.makeFlag(TimeUnit.QUARTER),
> f.timestampLiteral(ts), f.timestampLiteral(ts)));
> }
> {code}
> like
> {noformat}
> cannot convert TIMESTAMP literal to class
> org.apache.calcite.avatica.util.TimeUnit
> java.lang.AssertionError: cannot convert TIMESTAMP literal to class
> org.apache.calcite.avatica.util.TimeUnit
> at org.apache.calcite.rex.RexLiteral.getValueAs(RexLiteral.java:1143)
> at
> org.apache.calcite.rex.RexCallBinding.getOperandLiteralValue(RexCallBinding.java:100)
> at
> org.apache.calcite.sql.fun.SqlTimestampDiffFunction.inferReturnType2(SqlTimestampDiffFunction.java:69)
> at
> org.apache.calcite.sql.SqlOperator.inferReturnType(SqlOperator.java:537)
> at
> org.apache.calcite.rex.RexBuilder.deriveReturnType(RexBuilder.java:292)
> at org.apache.calcite.rex.RexBuilder.makeCall(RexBuilder.java:266)
> at
> org.apache.calcite.rex.RexBuilderTest.testTimestampDiffCall(RexBuilderTest.java:863)
> ...
> {noformat}
> It seems it recognise {{FLAG(QUARTER)}} as a literal...
> at {{org.apache.calcite.sql.fun.SqlTimestampDiffFunction#inferReturnType2}}
> {code:java}
> if (opBinding.isOperandLiteral(0, true)) {
> type1 = opBinding.getOperandType(0);
> type2 = opBinding.getOperandType(1);
> timeUnit = opBinding.getOperandLiteralValue(2, TimeUnit.class);
> } else {
> timeUnit = opBinding.getOperandLiteralValue(0, TimeUnit.class);
> type1 = opBinding.getOperandType(1);
> type2 = opBinding.getOperandType(2);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
