This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 02b67eae841c093d966bdb6371381be234c275d6 Author: TJ Banghart <[email protected]> AuthorDate: Fri Oct 28 18:55:38 2022 +0000 [CALCITE-5349] RelJson deserialization should support SqlLibraryOperators In RelJson, add create() method and deprecate constructor; add methods withOperatorTable, withLibraryOperatorTable, withJsonBuilder to further control its behavior. Add argument to RelJsonWriter and RelJsonReader constructors to allow configuring their embedded RelJson object. Close apache/calcite#2954 --- .../apache/calcite/rel/externalize/RelJson.java | 54 +++++++++++++++++++--- .../calcite/rel/externalize/RelJsonReader.java | 11 ++++- .../calcite/rel/externalize/RelJsonWriter.java | 16 ++++++- .../apache/calcite/plan/RelOptPlanReaderTest.java | 2 +- .../org/apache/calcite/plan/RelWriterTest.java | 31 ++++++++++++- 5 files changed, 100 insertions(+), 14 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 index f3460b4c59..3fd1aa32b6 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 @@ -54,7 +54,10 @@ import org.apache.calcite.sql.SqlIdentifier; import org.apache.calcite.sql.SqlIntervalQualifier; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlOperator; +import org.apache.calcite.sql.SqlOperatorTable; import org.apache.calcite.sql.SqlSyntax; +import org.apache.calcite.sql.fun.SqlLibrary; +import org.apache.calcite.sql.fun.SqlLibraryOperatorTableFactory; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; import org.apache.calcite.sql.type.SqlTypeName; @@ -93,6 +96,7 @@ public class RelJson { private final Map<String, Constructor> constructorMap = new HashMap<>(); private final @Nullable JsonBuilder jsonBuilder; private final InputTranslator inputTranslator; + private final SqlOperatorTable operatorTable; public static final List<String> PACKAGES = ImmutableList.of( @@ -104,14 +108,34 @@ public class RelJson { /** Private constructor. */ private RelJson(@Nullable JsonBuilder jsonBuilder, - InputTranslator inputTranslator) { + InputTranslator inputTranslator, SqlOperatorTable operatorTable) { this.jsonBuilder = jsonBuilder; this.inputTranslator = requireNonNull(inputTranslator, "inputTranslator"); + this.operatorTable = requireNonNull(operatorTable, "operatorTable"); } /** Creates a RelJson. */ + public static RelJson create() { + return new RelJson(null, RelJson::translateInput, + SqlStdOperatorTable.instance()); + } + + /** Creates a RelJson. + * + * @deprecated Use {@link RelJson#create}, followed by + * {@link #withJsonBuilder} if {@code jsonBuilder} is not null. */ + @Deprecated // to be removed before 2.0 public RelJson(@Nullable JsonBuilder jsonBuilder) { - this(jsonBuilder, RelJson::translateInput); + this(jsonBuilder, RelJson::translateInput, SqlStdOperatorTable.instance()); + } + + /** Returns a RelJson with a given JsonBuilder. */ + public RelJson withJsonBuilder(JsonBuilder jsonBuilder) { + requireNonNull(jsonBuilder, "jsonBuilder"); + if (jsonBuilder == this.jsonBuilder) { + return this; + } + return new RelJson(jsonBuilder, inputTranslator, operatorTable); } /** Returns a RelJson with a given InputTranslator. */ @@ -119,7 +143,23 @@ public class RelJson { if (inputTranslator == this.inputTranslator) { return this; } - return new RelJson(jsonBuilder, inputTranslator); + return new RelJson(jsonBuilder, inputTranslator, operatorTable); + } + + /** Returns a RelJson with a given operator table. */ + public RelJson withOperatorTable(SqlOperatorTable operatorTable) { + if (operatorTable == this.operatorTable) { + return this; + } + return new RelJson(jsonBuilder, inputTranslator, operatorTable); + } + + /** Returns a RelJson with an operator table that consists of the standard + * operators plus operators in all libraries. */ + public RelJson withLibraryOperatorTable() { + return withOperatorTable( + SqlLibraryOperatorTableFactory.INSTANCE.getOperatorTable( + SqlLibrary.values())); } private JsonBuilder jsonBuilder() { @@ -775,15 +815,15 @@ public class RelJson { String kind = get(map, "kind"); String syntax = get(map, "syntax"); SqlKind sqlKind = SqlKind.valueOf(kind); - SqlSyntax sqlSyntax = SqlSyntax.valueOf(syntax); + SqlSyntax sqlSyntax = SqlSyntax.valueOf(syntax); List<SqlOperator> operators = new ArrayList<>(); - SqlStdOperatorTable.instance().lookupOperatorOverloads( + operatorTable.lookupOperatorOverloads( new SqlIdentifier(name, new SqlParserPos(0, 0)), null, sqlSyntax, operators, SqlNameMatchers.liberal()); - for (SqlOperator operator: operators) { + for (SqlOperator operator : operators) { if (operator.kind == sqlKind) { return operator; } @@ -821,7 +861,7 @@ public class RelJson { public static RexNode readExpression(RelOptCluster cluster, InputTranslator translator, Map<String, Object> o) { RelInput relInput = new RelInputForCluster(cluster); - return new RelJson(null, translator).toRex(relInput, o); + return new RelJson(null, translator, SqlStdOperatorTable.instance()).toRex(relInput, o); } /** diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java index 165a29fe92..3c0820ad33 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonReader.java @@ -53,6 +53,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.function.UnaryOperator; import static java.util.Objects.requireNonNull; @@ -68,15 +69,21 @@ public class RelJsonReader { private final RelOptCluster cluster; private final RelOptSchema relOptSchema; - private final RelJson relJson = new RelJson(null); + private final RelJson relJson; private final Map<String, RelNode> relMap = new LinkedHashMap<>(); private @Nullable RelNode lastRel; public RelJsonReader(RelOptCluster cluster, RelOptSchema relOptSchema, Schema schema) { + this(cluster, relOptSchema, schema, UnaryOperator.identity()); + } + + public RelJsonReader(RelOptCluster cluster, RelOptSchema relOptSchema, + Schema schema, UnaryOperator<RelJson> relJsonTransform) { this.cluster = cluster; this.relOptSchema = relOptSchema; Util.discard(schema); + relJson = relJsonTransform.apply(RelJson.create()); } public RelNode read(String s) throws IOException { @@ -99,7 +106,7 @@ public class RelJsonReader { Map<String, Object> o = mapper .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true) .readValue(s, TYPE_REF); - return new RelJson(null).toType(typeFactory, o); + return RelJson.create().toType(typeFactory, o); } private void readRels(List<Map<String, Object>> jsonRels) { diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java index 964a945527..85d275f79d 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJsonWriter.java @@ -31,6 +31,9 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.UnaryOperator; + +import static java.util.Objects.requireNonNull; /** * Callback for a relational expression to dump itself as JSON. @@ -49,14 +52,23 @@ public class RelJsonWriter implements RelWriter { //~ Constructors ------------------------------------------------------------- + /** Creates a RelJsonWriter with a private JsonBuilder. */ public RelJsonWriter() { this(new JsonBuilder()); } + /** Creates a RelJsonWriter with a given JsonBuilder. */ public RelJsonWriter(JsonBuilder jsonBuilder) { - this.jsonBuilder = jsonBuilder; + this(jsonBuilder, UnaryOperator.identity()); + } + + /** Creates a RelJsonWriter. */ + public RelJsonWriter(JsonBuilder jsonBuilder, + UnaryOperator<RelJson> relJsonTransform) { + this.jsonBuilder = requireNonNull(jsonBuilder, "jsonBuilder"); relList = this.jsonBuilder.list(); - relJson = new RelJson(this.jsonBuilder); + relJson = + relJsonTransform.apply(RelJson.create().withJsonBuilder(jsonBuilder)); } //~ Methods ------------------------------------------------------------------ diff --git a/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java b/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java index b1ff290ad3..c274e3d177 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelOptPlanReaderTest.java @@ -34,7 +34,7 @@ import static org.junit.jupiter.api.Assertions.fail; */ class RelOptPlanReaderTest { @Test void testTypeToClass() { - RelJson relJson = new RelJson(null); + RelJson relJson = RelJson.create(); // in org.apache.calcite.rel package assertThat(relJson.classToTypeName(LogicalProject.class), 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 e51989d7da..c8aeaea431 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java @@ -54,6 +54,7 @@ import org.apache.calcite.schema.SchemaPlus; import org.apache.calcite.sql.SqlExplainFormat; import org.apache.calcite.sql.SqlExplainLevel; import org.apache.calcite.sql.SqlIntervalQualifier; +import org.apache.calcite.sql.fun.SqlLibraryOperators; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.fun.SqlTrimFunction; import org.apache.calcite.sql.parser.SqlParserPos; @@ -462,7 +463,7 @@ class RelWriterTest { .nullableRecord(false) .build(); final JsonBuilder jsonBuilder = new JsonBuilder(); - final RelJson json = new RelJson(jsonBuilder); + final RelJson json = RelJson.create().withJsonBuilder(jsonBuilder); final Object o = json.toJson(type); assertThat(o, notNullValue()); final String s = jsonBuilder.toJsonString(o); @@ -918,6 +919,31 @@ class RelWriterTest { + "No operator for 'MAXS' with kind: 'MAX', syntax: 'FUNCTION' during JSON deserialization"); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5349";>[CALCITE-5349] + * RelJson deserialization should support SqlLibraryOperators</a>. Before the + * fix, non-standard operators such as BigQuery's + * {@link SqlLibraryOperators#CURRENT_DATETIME} would throw during + * deserialization. */ + @Test void testDeserializeNonStandardOperator() { + final FrameworkConfig config = RelBuilderTest.config().build(); + final RelBuilder builder = RelBuilder.create(config); + final RelNode rel = builder + .scan("EMP") + .project(builder.field("JOB"), + builder.call(SqlLibraryOperators.CURRENT_DATETIME)) + .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=[CURRENT_DATETIME()])\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); @@ -1167,7 +1193,8 @@ class RelWriterTest { SqlExplainFormat format) { return Frameworks.withPlanner((cluster, relOptSchema, rootSchema) -> { final RelJsonReader reader = - new RelJsonReader(cluster, schema, rootSchema); + new RelJsonReader(cluster, schema, rootSchema, + RelJson::withLibraryOperatorTable); RelNode node; try { node = reader.read(relJson);
