Fix server SqlNode's and add unparse test to babel

Author: dssysolyatin

babel/src/main/codegen/config.fmpp

@@ -27,6 +27,7 @@ data: {
     # List of additional classes and packages to import.
     # Example: "org.apache.calcite.sql.*", "java.util.List".
     imports: [
+      "org.apache.calcite.schema.ColumnStrategy",
       "org.apache.calcite.sql.SqlCreate",
       "org.apache.calcite.sql.babel.SqlBabelCreateTable",
       "org.apache.calcite.sql.babel.TableCollectionType",

babel/src/main/codegen/includes/parserImpls.ftl

@@ -145,7 +145,7 @@ void ColumnWithType(List<SqlNode> list) :
     ]
     {
         list.add(SqlDdlNodes.column(s.add(id).end(this), id,
-            type.withNullable(nullable), null, null));
+            type.withNullable(nullable), null, ColumnStrategy.DEFAULT));
     }
 }
 

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) {

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.
@@ -390,7 +391,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")

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());
+  }
+}

core/src/main/java/org/apache/calcite/sql/SqlCreate.java

@@ -51,5 +51,4 @@ public boolean getReplace() {
   public void setReplace(boolean replace) {
     this.replace = replace;
   }
-
 }

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);
   }

core/src/main/java/org/apache/calcite/sql/ddl/SqlAttributeDefinition.java

@@ -23,23 +23,30 @@
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlSpecialOperator;
 import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.fun.SqlBasicOperator;
 import org.apache.calcite.sql.parser.SqlParserPos;
-
-import com.google.common.collect.ImmutableList;
+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 SqlAttributeDefinition,
  * which is part of a {@link SqlCreateType}.
  */
 public class SqlAttributeDefinition extends SqlCall {
-  private static final SqlSpecialOperator OPERATOR =
-      new SqlSpecialOperator("ATTRIBUTE_DEF", SqlKind.ATTRIBUTE_DEF);
+  private static final SqlOperator OPERATOR =
+      SqlBasicOperator.create("ATTRIBUTE_DEF", SqlKind.ATTRIBUTE_DEF)
+          .withCallFactory((operator, functionQualifier, pos, operands) ->
+              new SqlAttributeDefinition(
+                  pos,
+                  (SqlIdentifier) requireNonNull(operands[0]),
+                  (SqlDataTypeSpec) requireNonNull(operands[1]),
+                  operands[2], null));
 
   public final SqlIdentifier name;
   public final SqlDataTypeSpec dataType;
@@ -60,8 +67,9 @@ public class SqlAttributeDefinition extends SqlCall {
     return OPERATOR;
   }
 
+  @SuppressWarnings("nullness")
   @Override public List<SqlNode> getOperandList() {
-    return ImmutableList.of(name, dataType);
+    return ImmutableNullableList.of(name, dataType, expression);
   }
 
   @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {

core/src/main/java/org/apache/calcite/sql/ddl/SqlCheckConstraint.java

@@ -21,23 +21,30 @@
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlOperator;
-import org.apache.calcite.sql.SqlSpecialOperator;
 import org.apache.calcite.sql.SqlWriter;
+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 UNIQUE}, {@code PRIMARY KEY} constraints.
  *
  * <p>And {@code FOREIGN KEY}, when we support it.
  */
 public class SqlCheckConstraint extends SqlCall {
-  private static final SqlSpecialOperator OPERATOR =
-      new SqlSpecialOperator("CHECK", SqlKind.CHECK);
+  private static final SqlOperator OPERATOR =
+      SqlBasicOperator.create("CHECK", SqlKind.CHECK)
+          .withCallFactory((operator, functionQualifier, pos, operands) ->
+              new SqlCheckConstraint(
+                  pos,
+                  (SqlIdentifier) operands[0],
+                  requireNonNull(operands[1])));
 
   private final @Nullable SqlIdentifier name;
   private final SqlNode expression;

core/src/main/java/org/apache/calcite/sql/ddl/SqlColumnDeclaration.java

@@ -21,26 +21,37 @@
 import org.apache.calcite.sql.SqlDataTypeSpec;
 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.SqlOperator;
-import org.apache.calcite.sql.SqlSpecialOperator;
 import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.fun.SqlBasicOperator;
 import org.apache.calcite.sql.parser.SqlParserPos;
-
-import com.google.common.collect.ImmutableList;
+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 UNIQUE}, {@code PRIMARY KEY} constraints.
  *
  * <p>And {@code FOREIGN KEY}, when we support it.
  */
 public class SqlColumnDeclaration extends SqlCall {
-  private static final SqlSpecialOperator OPERATOR =
-      new SqlSpecialOperator("COLUMN_DECL", SqlKind.COLUMN_DECL);
+  private static final SqlOperator OPERATOR =
+      SqlBasicOperator.create("COLUMN_DECL", SqlKind.COLUMN_DECL)
+          .withCallFactory((operator, functionQualifier, pos, operands) ->
+              new SqlColumnDeclaration(
+                  pos,
+                  (SqlIdentifier) requireNonNull(operands[0]),
+                  (SqlDataTypeSpec) requireNonNull(operands[1]),
+                  operands[2],
+                  requireNonNull(
+                      ((SqlLiteral) requireNonNull(operands[3])).symbolValue(
+                          ColumnStrategy.class))));
 
   public final SqlIdentifier name;
   public final SqlDataTypeSpec dataType;
@@ -63,7 +74,8 @@ public class SqlColumnDeclaration extends SqlCall {
   }
 
   @Override public List<SqlNode> getOperandList() {
-    return ImmutableList.of(name, dataType);
+    return ImmutableNullableList.of(name, dataType, expression,
+        SqlLiteral.createSymbol(strategy, pos));
   }
 
   @Override public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {