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 511bef36872b..652f0a92e372 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 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 e3d49d2bb6e6..f458a174da6c 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 000000000000..99d6d2286e52 --- /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 cbed8e40a8de..ddadf32959e1 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 SqlMerge 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 3a7206eb2d99..689ba20642ba 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 528b9ee57db5..732e6d4e6a19 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 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 1bf283bf0cd6..51d4469dcf42 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 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 38297f83e6a3..53a688acfe2c 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 e84042b5cdcf..a2dc815ff3e8 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 ea6fd1372674..89d91bd2664c 100644 --- a/site/_docs/history.md +++ b/site/_docs/history.md @@ -49,6 +49,12 @@ other software versions as specified in gradle.properties. #### Breaking Changes {: #breaking-1-42-0} +* [CALCITE-7301] +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`. + * [CALCITE-6942] 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 613fe9b7de14..cfea5b6ce0e9 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 converter, List 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,