This is an automated email from the ASF dual-hosted git repository.
dmsysolyatin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new ab6a397c91 [CALCITE-7301] Add common test for SqlNode unparse after
deep copy and missing createCall in operators
ab6a397c91 is described below
commit ab6a397c9161824ef00715fba41eef54a39a083d
Author: Dmitry Sysolyatin <[email protected]>
AuthorDate: Wed Nov 19 14:25:12 2025 +0200
[CALCITE-7301] Add common test for SqlNode unparse after deep copy and
missing createCall in operators
---
.../calcite/sql/babel/SqlBabelCreateTable.java | 36 ++++++++++++++++++++--
.../org/apache/calcite/test/BabelParserTest.java | 8 ++++-
.../org/apache/calcite/test/BabelUnParserTest.java | 30 ++++++++++++++++++
.../main/java/org/apache/calcite/sql/SqlMerge.java | 13 +++++++-
.../java/org/apache/calcite/sql/SqlSetOption.java | 2 +-
.../java/org/apache/calcite/sql/SqlUnpivot.java | 16 ++++++++--
.../org/apache/calcite/sql/ddl/SqlCreateTable.java | 18 +++++++----
.../org/apache/calcite/sql/ddl/SqlCreateView.java | 4 +--
.../apache/calcite/sql/fun/SqlBasicOperator.java | 15 ++++++---
site/_docs/history.md | 6 ++++
.../apache/calcite/sql/parser/SqlParserTest.java | 22 +++++++++++++
11 files changed, 149 insertions(+), 21 deletions(-)
diff --git
a/babel/src/main/java/org/apache/calcite/sql/babel/SqlBabelCreateTable.java
b/babel/src/main/java/org/apache/calcite/sql/babel/SqlBabelCreateTable.java
index 511bef3687..652f0a92e3 100644
--- a/babel/src/main/java/org/apache/calcite/sql/babel/SqlBabelCreateTable.java
+++ b/babel/src/main/java/org/apache/calcite/sql/babel/SqlBabelCreateTable.java
@@ -17,17 +17,39 @@
package org.apache.calcite.sql.babel;
import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.ddl.SqlCreateTable;
+import org.apache.calcite.sql.fun.SqlBasicOperator;
import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.util.ImmutableNullableList;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
/**
* Parse tree for {@code CREATE TABLE} statement, with extensions for
particular
* SQL dialects supported by Babel.
*/
public class SqlBabelCreateTable extends SqlCreateTable {
+ private static final SqlOperator OPERATOR =
+ SqlBasicOperator.create("CREATE TABLE",
SqlKind.CREATE_TABLE).withCallFactory(
+ (operator, functionQualifier, pos, operands) ->
+ new SqlBabelCreateTable(pos,
+ requireNonNull((SqlLiteral) operands[0]).booleanValue(),
+ requireNonNull((SqlLiteral)
operands[1]).symbolValue(TableCollectionType.class),
+ requireNonNull((SqlLiteral) operands[2]).booleanValue(),
+ requireNonNull((SqlLiteral) operands[3]).booleanValue(),
+ (SqlIdentifier) requireNonNull(operands[4]),
+ (SqlNodeList) operands[5], operands[6]));
+
private final TableCollectionType tableCollectionType;
// CHECKSTYLE: IGNORE 2; can't use 'volatile' because it is a Java keyword
// but checkstyle does not like trailing '_'.
@@ -36,13 +58,21 @@ public class SqlBabelCreateTable extends SqlCreateTable {
/** Creates a SqlBabelCreateTable. */
public SqlBabelCreateTable(SqlParserPos pos, boolean replace,
TableCollectionType tableCollectionType, boolean volatile_,
- boolean ifNotExists, SqlIdentifier name, SqlNodeList columnList,
- SqlNode query) {
- super(pos, replace, ifNotExists, name, columnList, query);
+ boolean ifNotExists, SqlIdentifier name, @Nullable SqlNodeList
columnList,
+ @Nullable SqlNode query) {
+ super(OPERATOR, pos, replace, ifNotExists, name, columnList, query);
this.tableCollectionType = tableCollectionType;
this.volatile_ = volatile_;
}
+ @SuppressWarnings("nullness")
+ @Override public List<SqlNode> getOperandList() {
+ return ImmutableNullableList.of(SqlLiteral.createBoolean(getReplace(),
pos),
+ SqlLiteral.createSymbol(tableCollectionType, pos),
+ SqlLiteral.createBoolean(volatile_, pos),
+ SqlLiteral.createBoolean(ifNotExists, pos), name, columnList, query);
+ }
+
@Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec)
{
writer.keyword("CREATE");
switch (tableCollectionType) {
diff --git a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
index e3d49d2bb6..f458a174da 100644
--- a/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
+++ b/babel/src/test/java/org/apache/calcite/test/BabelParserTest.java
@@ -40,6 +40,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasToString;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
/**
* Tests the "Babel" SQL parser, that understands all dialects of SQL.
@@ -415,7 +416,12 @@ private void checkParseInfixCast(String sqlType) {
}
@Test void testPostgresSqlSetOption() {
- SqlParserFixture f = fixture().withDialect(PostgresqlSqlDialect.DEFAULT);
+ // UnparsingTesterImpl has a check where it unparses a SqlNode into a SQL
string
+ // using the calcite dialect, and then parses it back into a SqlNode.
+ // But the SQL string produced by the calcite dialect for `SET` cannot
always be parsed back.
+ assumeFalse(fixture().tester.isUnparserTest());
+ SqlParserFixture f = fixture()
+ .withDialect(PostgresqlSqlDialect.DEFAULT);
f.sql("SET SESSION autovacuum = true")
.ok("SET \"autovacuum\" = TRUE");
f.sql("SET SESSION autovacuum = DEFAULT")
diff --git a/babel/src/test/java/org/apache/calcite/test/BabelUnParserTest.java
b/babel/src/test/java/org/apache/calcite/test/BabelUnParserTest.java
new file mode 100644
index 0000000000..99d6d2286e
--- /dev/null
+++ b/babel/src/test/java/org/apache/calcite/test/BabelUnParserTest.java
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test;
+
+import org.apache.calcite.sql.parser.SqlParserFixture;
+
+/**
+ * Extension to {@link BabelParserTest} that ensures that every expression can
+ * un-parse successfully.
+ */
+public class BabelUnParserTest extends BabelParserTest {
+ @Override public SqlParserFixture fixture() {
+ return super.fixture()
+ .withTester(new UnparsingTesterImpl());
+ }
+}
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlMerge.java
b/core/src/main/java/org/apache/calcite/sql/SqlMerge.java
index cbed8e40a8..ddadf32959 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlMerge.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlMerge.java
@@ -28,13 +28,24 @@
import java.util.List;
+import static java.util.Objects.requireNonNull;
+
/**
* A <code>SqlMerge</code> is a node of a parse tree which represents a MERGE
* statement.
*/
public class SqlMerge extends SqlCall {
public static final SqlSpecialOperator OPERATOR =
- new SqlSpecialOperator("MERGE", SqlKind.MERGE);
+ new SqlSpecialOperator("MERGE", SqlKind.MERGE) {
+ @Override public SqlCall createCall(final @Nullable SqlLiteral
functionQualifier,
+ final SqlParserPos pos,
+ final @Nullable SqlNode... operands) {
+ return new SqlMerge(pos, requireNonNull(operands[0]),
requireNonNull(operands[1]),
+ requireNonNull(operands[2]),
+ (SqlUpdate) operands[3], (SqlInsert) operands[4],
+ (SqlSelect) operands[5], (SqlIdentifier) operands[6]);
+ }
+ };
SqlNode targetTable;
SqlNode condition;
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
b/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
index 3a7206eb2d..689ba20642 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlSetOption.java
@@ -143,7 +143,7 @@ public SqlSetOption(SqlParserPos pos, @Nullable String
scope, SqlIdentifier name
} else {
operandList.add(new SqlIdentifier(scope, SqlParserPos.ZERO));
}
- operandList.add(name);
+ operandList.add(nameAsSqlNode);
operandList.add(value);
return ImmutableNullableList.copyOf(operandList);
}
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlUnpivot.java
b/core/src/main/java/org/apache/calcite/sql/SqlUnpivot.java
index 528b9ee57d..732e6d4e6a 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlUnpivot.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlUnpivot.java
@@ -67,10 +67,10 @@ public SqlUnpivot(SqlParserPos pos, SqlNode query, boolean
includeNulls,
SqlNodeList measureList, SqlNodeList axisList, SqlNodeList inList) {
super(pos);
this.query = requireNonNull(query, "query");
- this.includeNulls = includeNulls;
this.measureList = requireNonNull(measureList, "measureList");
this.axisList = requireNonNull(axisList, "axisList");
this.inList = requireNonNull(inList, "inList");
+ this.includeNulls = includeNulls;
}
//~ Methods ----------------------------------------------------------------
@@ -80,7 +80,8 @@ public SqlUnpivot(SqlParserPos pos, SqlNode query, boolean
includeNulls,
}
@Override public List<SqlNode> getOperandList() {
- return ImmutableNullableList.of(query, measureList, axisList, inList);
+ return ImmutableNullableList.of(query, measureList, axisList, inList,
+ SqlLiteral.createBoolean(includeNulls, SqlParserPos.ZERO));
}
@SuppressWarnings("nullness")
@@ -176,5 +177,16 @@ static class Operator extends SqlSpecialOperator {
Operator(SqlKind kind) {
super(kind.name(), kind);
}
+
+ @Override public SqlCall createCall(
+ @Nullable SqlLiteral functionQualifier,
+ SqlParserPos pos,
+ @Nullable SqlNode... operands) {
+ return new SqlUnpivot(pos, requireNonNull(operands[0]),
+ requireNonNull((SqlLiteral) operands[4]).booleanValue(),
+ requireNonNull((SqlNodeList) operands[1]),
+ requireNonNull((SqlNodeList) operands[2]),
+ requireNonNull((SqlNodeList) operands[3]));
+ }
}
}
diff --git a/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
b/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
index 1bf283bf0c..51d4469dcf 100644
--- a/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java
@@ -47,24 +47,30 @@ public class SqlCreateTable extends SqlCreate {
new SqlSpecialOperator("CREATE TABLE", SqlKind.CREATE_TABLE) {
@Override public SqlCall createCall(@Nullable SqlLiteral
functionQualifier,
SqlParserPos pos, @Nullable SqlNode... operands) {
- return new SqlCreateTable(pos,
+ return new SqlCreateTable(OPERATOR, pos,
((SqlLiteral) requireNonNull(operands[0],
"replace")).booleanValue(),
((SqlLiteral) requireNonNull(operands[1],
"ifNotExists")).booleanValue(),
(SqlIdentifier) requireNonNull(operands[2], "name"),
- (SqlNodeList) requireNonNull(operands[3], "columnList"),
- operands[4]);
+ (SqlNodeList) operands[3], operands[4]);
}
};
/** Creates a SqlCreateTable. */
- protected SqlCreateTable(SqlParserPos pos, boolean replace, boolean
ifNotExists,
- SqlIdentifier name, @Nullable SqlNodeList columnList, @Nullable SqlNode
query) {
- super(OPERATOR, pos, replace, ifNotExists);
+ protected SqlCreateTable(SqlOperator operator, SqlParserPos pos, boolean
replace,
+ boolean ifNotExists, SqlIdentifier name, @Nullable SqlNodeList
columnList,
+ @Nullable SqlNode query) {
+ super(operator, pos, replace, ifNotExists);
this.name = requireNonNull(name, "name");
this.columnList = columnList; // may be null
this.query = query; // for "CREATE TABLE ... AS query"; may be null
}
+ /** Creates a SqlCreateTable. */
+ protected SqlCreateTable(SqlParserPos pos, boolean replace, boolean
ifNotExists,
+ SqlIdentifier name, @Nullable SqlNodeList columnList, @Nullable SqlNode
query) {
+ this(OPERATOR, pos, replace, ifNotExists, name, columnList, query);
+ }
+
@SuppressWarnings("nullness")
@Override public List<SqlNode> getOperandList() {
return ImmutableNullableList.of(
diff --git a/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateView.java
b/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateView.java
index 38297f83e6..53a688acfe 100644
--- a/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateView.java
+++ b/core/src/main/java/org/apache/calcite/sql/ddl/SqlCreateView.java
@@ -50,8 +50,8 @@ public class SqlCreateView extends SqlCreate {
return new SqlCreateView(pos,
((SqlLiteral) requireNonNull(operands[0],
"replace")).booleanValue(),
(SqlIdentifier) requireNonNull(operands[1], "name"),
- (SqlNodeList) operands[3],
- requireNonNull(operands[4], "query"));
+ (SqlNodeList) operands[2],
+ requireNonNull(operands[3], "query"));
}
};
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicOperator.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicOperator.java
index e84042b5cd..a2dc815ff3 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlBasicOperator.java
@@ -40,24 +40,29 @@ public final class SqlBasicOperator extends SqlOperator {
}
/** Private constructor. Use {@link #create}. */
- private SqlBasicOperator(String name, int leftPrecedence, int
rightPrecedence,
+ private SqlBasicOperator(String name, SqlKind kind, int leftPrecedence, int
rightPrecedence,
SqlCallFactory callFactory) {
- super(name, SqlKind.OTHER, leftPrecedence, rightPrecedence,
+ super(name, kind, leftPrecedence, rightPrecedence,
ReturnTypes.BOOLEAN, InferTypes.RETURN_TYPE, OperandTypes.ANY,
callFactory);
}
public static SqlBasicOperator create(String name) {
- return new SqlBasicOperator(name, 0, 0,
+ return new SqlBasicOperator(name, SqlKind.OTHER, 0, 0,
+ SqlCallFactories.SQL_BASIC_CALL_FACTORY);
+ }
+
+ public static SqlBasicOperator create(String name, SqlKind kind) {
+ return new SqlBasicOperator(name, kind, 0, 0,
SqlCallFactories.SQL_BASIC_CALL_FACTORY);
}
public SqlBasicOperator withPrecedence(int prec, boolean leftAssoc) {
- return new SqlBasicOperator(getName(), leftPrec(prec, leftAssoc),
+ return new SqlBasicOperator(getName(), getKind(), leftPrec(prec,
leftAssoc),
rightPrec(prec, leftAssoc), getSqlCallFactory());
}
public SqlBasicOperator withCallFactory(SqlCallFactory sqlCallFactory) {
- return new SqlBasicOperator(getName(), getLeftPrec(),
+ return new SqlBasicOperator(getName(), getKind(), getLeftPrec(),
getRightPrec(), sqlCallFactory);
}
}
diff --git a/site/_docs/history.md b/site/_docs/history.md
index ea6fd13726..89d91bd266 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -49,6 +49,12 @@ ## <a
href="https://github.com/apache/calcite/releases/tag/calcite-1.42.0";>1.42.
#### Breaking Changes
{: #breaking-1-42-0}
+* [<a
href="https://issues.apache.org/jira/browse/CALCITE-7301";>CALCITE-7301</a>]
+Prior to this change, most `SqlNode`s in the `org.apache.calcite.sql.ddl`
package could not be unparsed
+when created with `SqlOperator#createCall`. To fix this, those `SqlNode`s now
implement their own `SqlOperator`.
+`SqlNode#getOperandList()` now returns all operands required by these
operators; the number and order may differ from before.
+The same applies to `SqlBabelCreateTable` and `SqlUnpivot`.
+
* [<a
href="https://issues.apache.org/jira/browse/CALCITE-6942";>CALCITE-6942</a>]
Rename the method `decorrelateFetchOneSort` to `decorrelateSortWithRowNumber`.
diff --git
a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 613fe9b7de..cfea5b6ce0 100644
--- a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -9886,6 +9886,17 @@ static void checkList(SqlNodeList sqlNodeList,
}
}
+ static SqlNode deepCopy(SqlNode sqlNode) {
+ return sqlNode.accept(new SqlShuttle() {
+ @Override public @Nullable SqlNode visit(final SqlCall call) {
+ // Handler always creates a new copy of 'call'
+ CallCopyingArgHandler argHandler = new CallCopyingArgHandler(call,
true);
+ call.getOperator().acceptCall(this, call, false, argHandler);
+ return argHandler.result();
+ }
+ });
+ }
+
@Override public void checkList(SqlTestFactory factory, StringAndPos sap,
@Nullable SqlDialect dialect, UnaryOperator<String> converter,
List<String> expected) {
@@ -9915,6 +9926,12 @@ static void checkList(SqlNodeList sqlNodeList,
final Random random = new Random();
final String sql3 = toSqlString(sqlNodeList, randomize(random));
assertThat(sql3, notNullValue());
+
+ // Make a deep copy of the SqlNodeList, unparse it.
+ final SqlNodeList sqlNodeList3 = (SqlNodeList) deepCopy(sqlNodeList);
+ final String sql4 = toSqlString(sqlNodeList3, simple());
+ // Should be the same as we started with.
+ assertThat(sql4, is(sql1));
}
@Override public void check(SqlTestFactory factory, StringAndPos sap,
@@ -9959,6 +9976,11 @@ static void checkList(SqlNodeList sqlNodeList,
parseStmtAndHandleEx(factory2, sql1, parser -> { });
final String sql4 = sqlNode4.toSqlString(simple()).getSql();
assertThat(sql4, is(sql1));
+
+ // Make a deep copy of the original SqlNode, unparse it.
+ final SqlNode sqlNode5 = deepCopy(sqlNode);
+ final String actual5 = sqlNode5.toSqlString(writerTransform).getSql();
+ assertThat(converter.apply(actual5), is(expected));
}
@Override public void checkExp(SqlTestFactory factory, StringAndPos sap,
