You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/01/27 00:53:42 UTC
[calcite] 01/02: [CALCITE-5349] RelJson deserialization should support SqlLibraryOperators
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 <tj...@gmail.com>
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);