[CALCITE-5607] WIP: Serialize return type during RelJson.toJson(RexNo…

…de node) (#28) * Add tests to fix later * Update test * Add in type field to RelJson.toJson and update tests * CALCITE-5607 / Serialize return type during RelJson.toJson(RexNode node) * Update test comment --------- Co-authored-by: Oliver Lee <[email protected]>
looker-open-source · Mar 24, 2023 · dc68013 · dc68013 · tjbanghart · Mar 28, 2023
1 parent 49cac0f
commit dc68013
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 18 deletions.
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
@@ -554,31 +554,23 @@ private Object toJson(RexNode node) {
         final RexCall call = (RexCall) node;
         map = jsonBuilder().map();
         map.put("op", toJson(call.getOperator()));
+        map.put("type", toJson(call.getType()));
         final List<@Nullable Object> list = jsonBuilder().list();
         for (RexNode operand : call.getOperands()) {
           list.add(toJson(operand));
         }
         map.put("operands", list);
-        switch (node.getKind()) {
-        case CAST:
-          map.put("type", toJson(node.getType()));
-          break;
-        default:
-          break;
-        }
         if (call.getOperator() instanceof SqlFunction) {
           if (((SqlFunction) call.getOperator()).getFunctionType().isUserDefined()) {
             SqlOperator op = call.getOperator();
             map.put("class", op.getClass().getName());
-            map.put("type", toJson(node.getType()));
             map.put("deterministic", op.isDeterministic());
             map.put("dynamic", op.isDynamicFunction());
           }
         }
         if (call instanceof RexOver) {
           RexOver over = (RexOver) call;
           map.put("distinct", over.isDistinct());
-          map.put("type", toJson(node.getType()));
           map.put("window", toJson(over.getWindow()));
         }
         return map;

diff --git a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
@@ -129,6 +129,10 @@ class RelWriterTest {
       + "          \"kind\": \"EQUALS\",\n"
       + "          \"syntax\": \"BINARY\"\n"
       + "        },\n"
+      + "        \"type\": {\n"
+      + "          \"type\": \"BOOLEAN\",\n"
+      + "          \"nullable\": false\n"
+      + "        },\n"
       + "        \"operands\": [\n"
       + "          {\n"
       + "            \"input\": 1,\n"
@@ -288,17 +292,17 @@ class RelWriterTest {
       + "            \"kind\": \"COUNT\",\n"
       + "            \"syntax\": \"FUNCTION_STAR\"\n"
       + "          },\n"
+      + "          \"type\": {\n"
+      + "            \"type\": \"BIGINT\",\n"
+      + "            \"nullable\": false\n"
+      + "          },\n"
       + "          \"operands\": [\n"
       + "            {\n"
       + "              \"input\": 0,\n"
       + "              \"name\": \"$0\"\n"
       + "            }\n"
       + "          ],\n"
       + "          \"distinct\": false,\n"
-      + "          \"type\": {\n"
-      + "            \"type\": \"BIGINT\",\n"
-      + "            \"nullable\": false\n"
-      + "          },\n"
       + "          \"window\": {\n"
       + "            \"partition\": [\n"
       + "              {\n"
@@ -330,17 +334,17 @@ class RelWriterTest {
       + "            \"kind\": \"SUM\",\n"
       + "            \"syntax\": \"FUNCTION\"\n"
       + "          },\n"
+      + "          \"type\": {\n"
+      + "            \"type\": \"BIGINT\",\n"
+      + "            \"nullable\": false\n"
+      + "          },\n"
       + "          \"operands\": [\n"
       + "            {\n"
       + "              \"input\": 0,\n"
       + "              \"name\": \"$0\"\n"
       + "            }\n"
       + "          ],\n"
       + "          \"distinct\": false,\n"
-      + "          \"type\": {\n"
-      + "            \"type\": \"BIGINT\",\n"
-      + "            \"nullable\": false\n"
-      + "          },\n"
       + "          \"window\": {\n"
       + "            \"partition\": [\n"
       + "              {\n"
@@ -779,7 +783,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.toString(), is(matcher));
   }
@@ -945,6 +950,42 @@ 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 deserialization.
+   * */
+  @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);