Skip to content

Commit

Permalink
[CALCITE-5607] Serialize return type during RelJson.toJson(RexNode no…
Browse files Browse the repository at this point in the history
…de) for SqlKind.MINUS

Uncovered a bug in RelJson#toRex for the TIMESTAMP_DIFF call for Big Query dialect.
MINUS_DATE uses an ARG2_NULLABLE return type inference which requires 3 operands, however there are only 2 operands for BQ.
The solution here is to do something similar to how we handle CAST and to add in "type" when serializing to JSON in RelJson.toJson(RexNode node) for SqlKind.MINUS so that jsonType will be defined in toRex.

Co-authored-by: TJ Banghart <[email protected]>
  • Loading branch information
olivrlee and tjbanghart committed Nov 15, 2023
1 parent 5503451 commit 943921e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,7 @@ public Object toJson(RexNode node) {
}
map.put("operands", list);
switch (node.getKind()) {
case MINUS:
case CAST:
map.put("type", toJson(node.getType()));
break;
Expand Down
40 changes: 39 additions & 1 deletion core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,8 @@ private void assertThatReadExpressionResult(String json, Matcher<String> matcher
throw TestUtil.rethrow(e);
}
final RelJson relJson = RelJson.create()
.withInputTranslator(RelWriterTest::translateInput);
.withInputTranslator(RelWriterTest::translateInput)
.withLibraryOperatorTable();
final RexNode e = relJson.toRex(cluster, o);
assertThat(e, hasToString(matcher));
}
Expand Down Expand Up @@ -1052,6 +1053,43 @@ void testAggregateWithAlias(SqlExplainFormat format) {
assertThat(result, isLinux(expected));
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-5607">[CALCITE-5607]</a>
*
* <p>Before the fix, RelJson.toRex would throw an ArrayIndexOutOfBounds error
* when deserializing MINUS_DATE due to type inference requiring 3 operands.
*
* <p>The solution is to add in 'type' when serializing to JSON.
*/
@Test void testDeserializeMinusDateOperator() {
final FrameworkConfig config = RelBuilderTest.config().build();
final RelBuilder builder = RelBuilder.create(config);
final RexBuilder rb = builder.getRexBuilder();
final RelDataTypeFactory typeFactory = rb.getTypeFactory();
final SqlIntervalQualifier qualifier =
new SqlIntervalQualifier(TimeUnit.MONTH, null, SqlParserPos.ZERO);
final RexNode op1 = rb.makeTimestampLiteral(new TimestampString("2012-12-03 12:34:44"), 0);
final RexNode op2 = rb.makeTimestampLiteral(new TimestampString("2014-12-23 12:34:44"), 0);
final RelDataType intervalType =
typeFactory.createTypeWithNullability(
typeFactory.createSqlIntervalType(qualifier),
op1.getType().isNullable() || op2.getType().isNullable());
final RelNode rel = builder
.scan("EMP")
.project(builder.field("JOB"),
rb.makeCall(intervalType, SqlStdOperatorTable.MINUS_DATE,
ImmutableList.of(op2, op1))).build();
final RelJsonWriter jsonWriter =
new RelJsonWriter(new JsonBuilder(), RelJson::withLibraryOperatorTable);
rel.explain(jsonWriter);
String relJson = jsonWriter.asString();
String result = deserializeAndDumpToTextFormat(getSchema(rel), relJson);
final String expected =
"LogicalProject(JOB=[$2], $f1=[-(2014-12-23 12:34:44, 2012-12-03 12:34:44)])\n"
+ " LogicalTableScan(table=[[scott, EMP]])\n";
assertThat(result, isLinux(expected));
}

@Test void testAggregateWithoutAlias() {
final FrameworkConfig config = RelBuilderTest.config().build();
final RelBuilder builder = RelBuilder.create(config);
Expand Down

0 comments on commit 943921e

Please sign in to comment.