From 943921e4941282457c7b48ba0a8cd6075ddb82d7 Mon Sep 17 00:00:00 2001 From: Oliver Lee Date: Tue, 14 Mar 2023 15:15:21 -0700 Subject: [PATCH] [CALCITE-5607] Serialize return type during RelJson.toJson(RexNode node) 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 --- .../calcite/rel/externalize/RelJson.java | 1 + .../apache/calcite/plan/RelWriterTest.java | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java index e915917c918..81978283dae 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java @@ -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; diff --git a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java index 4a1c05d24a0..f09ec0e24f3 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java @@ -786,7 +786,8 @@ private void assertThatReadExpressionResult(String json, Matcher 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)); } @@ -1052,6 +1053,43 @@ void testAggregateWithAlias(SqlExplainFormat format) { assertThat(result, isLinux(expected)); } + /** Test case for + * [CALCITE-5607] + * + *

Before the fix, RelJson.toRex would throw an ArrayIndexOutOfBounds error + * when deserializing MINUS_DATE due to type inference requiring 3 operands. + * + *

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);