[CALCITE-3392] Column expression in DDL should be validated before co…

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java

@@ -16,11 +16,15 @@
  */
 package org.apache.calcite.sql.validate;
 
+import org.apache.calcite.config.CalciteConnectionConfig;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
 import org.apache.calcite.jdbc.CalciteSchema;
 import org.apache.calcite.linq4j.Linq4j;
 import org.apache.calcite.linq4j.Ord;
 import org.apache.calcite.plan.RelOptSchemaWithSampling;
 import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.prepare.CalciteCatalogReader;
 import org.apache.calcite.prepare.Prepare;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.type.RelDataType;
@@ -30,6 +34,8 @@
 import org.apache.calcite.schema.CustomColumnResolvingTable;
 import org.apache.calcite.schema.ExtensibleTable;
 import org.apache.calcite.schema.Table;
+import org.apache.calcite.schema.impl.AbstractSchema;
+import org.apache.calcite.schema.impl.AbstractTable;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlDataTypeSpec;
 import org.apache.calcite.sql.SqlDynamicParam;
@@ -41,6 +47,7 @@
 import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.SqlSelect;
 import org.apache.calcite.sql.SqlSyntax;
 import org.apache.calcite.sql.SqlUtil;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
@@ -60,11 +67,14 @@
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -1118,6 +1128,89 @@ public static SqlOperator lookupSqlFunctionByID(SqlOperatorTable opTab,
     return null;
   }
 
+  /**
+   * Validate the sql node with specified base table row type. For "base table", we mean the
+   * table that the sql node expression references fields with.
+   *
+   * @param caseSensitive whether to match the catalog case-sensitively
+   * @param operatorTable operator table
+   * @param typeFactory   type factory
+   * @param rowType       the table row type that has fields referenced by the expression
+   * @param expr          the expression to validate
+   * @return pair of a validated expression sql node and its data type,
+   * usually a SqlUnresolvedFunction is converted to a resolved function
+   */
+  public static Pair<SqlNode, RelDataType> validateExprWithRowType(
+      boolean caseSensitive,
+      SqlOperatorTable operatorTable,
+      RelDataTypeFactory typeFactory,
+      RelDataType rowType,
+      SqlNode expr) {
+    final String tableName = "_table_";
+    final SqlSelect select0 = new SqlSelect(SqlParserPos.ZERO, null,
+        new SqlNodeList(Collections.singletonList(expr), SqlParserPos.ZERO),
+        new SqlIdentifier(tableName, SqlParserPos.ZERO),
+        null, null, null, null, null, null, null);
+    Prepare.CatalogReader catalogReader = createSingleTableCatalogReader(
+        caseSensitive,
+        tableName,
+        typeFactory,
+        rowType);
+    SqlValidator validator = newValidator(operatorTable,
+        catalogReader,
+        typeFactory,
+        SqlConformanceEnum.DEFAULT);
+    final SqlSelect select = (SqlSelect) validator.validate(select0);
+    assert select.getSelectList().size() == 1
+        : "Expression " + expr + " should be atom expression";
+    final SqlNode node = select.getSelectList().get(0);
+    final RelDataType nodeType = validator
+        .getValidatedNodeType(select)
+        .getFieldList()
+        .get(0).getType();
+    return Pair.of(node, nodeType);
+  }
+
+  /**
+   * Creates a catalog reader that contains a single {@link Table} with temporary table name
+   * and specified {@code rowType}.
+   *
+   * <p>Make this method public so that other systems can also use it.
+   *
+   * @param caseSensitive whether to match case sensitively
+   * @param tableName     table name to register with
+   * @param typeFactory   type factory
+   * @param rowType       table row type
+   * @return the {@link CalciteCatalogReader} instance
+   */
+  public static CalciteCatalogReader createSingleTableCatalogReader(
+      boolean caseSensitive,
+      String tableName,
+      RelDataTypeFactory typeFactory,
+      RelDataType rowType) {
+    // connection properties
+    Properties properties = new Properties();
+    properties.put(
+        CalciteConnectionProperty.CASE_SENSITIVE.camelName(),
+        String.valueOf(caseSensitive));
+    CalciteConnectionConfig connectionConfig = new CalciteConnectionConfigImpl(properties);
+
+    // prepare root schema
+    final ExplicitRowTypeTable table = new ExplicitRowTypeTable(rowType);
+    final Map<String, Table> tableMap = Collections.singletonMap(tableName, table);
+    CalciteSchema schema = CalciteSchema.createRootSchema(
+        false,
+        false,
+        "",
+        new ExplicitTablesSchema(tableMap));
+
+    return new CalciteCatalogReader(
+        schema,
+        new ArrayList<>(new ArrayList<>()),
+        typeFactory,
+        connectionConfig);
+  }
+
   //~ Inner Classes ----------------------------------------------------------
 
   /**
@@ -1196,11 +1289,7 @@ public interface Suggester {
           + Math.max(size, attempt);
 
   public static final Suggester ATTEMPT_SUGGESTER =
-      new Suggester() {
-        public String apply(String original, int attempt, int size) {
-          return Util.first(original, "$") + attempt;
-        }
-      };
+      (original, attempt, size) -> Util.first(original, "$") + attempt;
 
   /** Builds a list of GROUP BY expressions. */
   static class GroupAnalyzer {
@@ -1221,6 +1310,36 @@ SqlNode createGroupExpr() {
           SqlParserPos.ZERO);
     }
   }
+
+  /**
+   * A {@link AbstractTable} that can specify the row type explicitly.
+   */
+  private static class ExplicitRowTypeTable extends AbstractTable {
+    private final RelDataType rowType;
+
+    ExplicitRowTypeTable(RelDataType rowType) {
+      this.rowType = Objects.requireNonNull(rowType);
+    }
+
+    @Override public RelDataType getRowType(RelDataTypeFactory typeFactory) {
+      return this.rowType;
+    }
+  }
+
+  /**
+   * A {@link AbstractSchema} that can specify the table map explicitly.
+   */
+  private static class ExplicitTablesSchema extends AbstractSchema {
+    private final Map<String, Table> tableMap;
+
+    ExplicitTablesSchema(Map<String, Table> tableMap) {
+      this.tableMap = Objects.requireNonNull(tableMap);
+    }
+
+    @Override protected Map<String, Table> getTableMap() {
+      return tableMap;
+    }
+  }
 }
 
 // End SqlValidatorUtil.java

core/src/main/java/org/apache/calcite/sql2rel/InitializerContext.java

@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.sql2rel;
 
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.sql.SqlNode;
@@ -26,6 +27,8 @@
 public interface InitializerContext {
   RexBuilder getRexBuilder();
 
+  SqlNode validateExpression(RelDataType rowType, SqlNode expr);
+
   RexNode convertExpression(SqlNode e);
 }
 

core/src/main/java/org/apache/calcite/sql2rel/InitializerExpressionFactory.java

@@ -62,6 +62,10 @@ ColumnStrategy generationStrategy(
    * Creates an expression which evaluates to the default value for a
    * particular column.
    *
+   * <p>If the default value comes from a un-validated {@link org.apache.calcite.sql.SqlNode},
+   * make sure to invoke {@link InitializerContext#validateExpression} first before you actually
+   * do the conversion with method {@link InitializerContext#convertExpression}.
+   *
    * @param table   the table containing the column
    * @param iColumn the 0-based offset of the column in the table
    * @param context Context for creating the expression

core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java

@@ -3331,34 +3331,36 @@ public RelNode toRel(final RelOptTable table) {
         Util.first(table.unwrap(InitializerExpressionFactory.class),
             NullInitializerExpressionFactory.INSTANCE);
 
-    // Lazily create a blackboard that contains all non-generated columns.
-    final Supplier<Blackboard> bb = () -> {
-      RexNode sourceRef = rexBuilder.makeRangeReference(scan);
-      return createInsertBlackboard(table, sourceRef,
-          table.getRowType().getFieldNames());
-    };
-
-    int virtualCount = 0;
-    final List<RexNode> list = new ArrayList<>();
-    for (RelDataTypeField f : table.getRowType().getFieldList()) {
-      final ColumnStrategy strategy =
-          ief.generationStrategy(table, f.getIndex());
-      switch (strategy) {
-      case VIRTUAL:
-        list.add(ief.newColumnDefaultValue(table, f.getIndex(), bb.get()));
-        ++virtualCount;
-        break;
-      default:
-        list.add(
-            rexBuilder.makeInputRef(scan,
-                RelOptTableImpl.realOrdinal(table, f.getIndex())));
+    boolean hasVirtualFields = table.getRowType()
+        .getFieldList().stream()
+        .anyMatch(f -> ief.generationStrategy(table, f.getIndex()) == ColumnStrategy.VIRTUAL);
+
+    if (hasVirtualFields) {
+      // Lazily create a blackboard that contains all non-generated columns.
+      final Supplier<Blackboard> bb = () -> {
+        RexNode sourceRef = rexBuilder.makeRangeReference(scan);
+        return createInsertBlackboard(table, sourceRef,
+            table.getRowType().getFieldNames());
+      };
+      final List<RexNode> list = new ArrayList<>();
+      for (RelDataTypeField f : table.getRowType().getFieldList()) {
+        final ColumnStrategy strategy =
+            ief.generationStrategy(table, f.getIndex());
+        switch (strategy) {
+        case VIRTUAL:
+          list.add(ief.newColumnDefaultValue(table, f.getIndex(), bb.get()));
+          break;
+        default:
+          list.add(
+              rexBuilder.makeInputRef(scan,
+                  RelOptTableImpl.realOrdinal(table, f.getIndex())));
+        }
       }
-    }
-    if (virtualCount > 0) {
       relBuilder.push(scan);
       relBuilder.project(list);
       return relBuilder.build();
     }
+
     return scan;
   }
 
@@ -4744,6 +4746,12 @@ public RexBuilder getRexBuilder() {
       return rexBuilder;
     }
 
+    public SqlNode validateExpression(RelDataType rowType, SqlNode expr) {
+      return SqlValidatorUtil.validateExprWithRowType(
+          catalogReader.nameMatcher().isCaseSensitive(), opTab,
+          typeFactory, rowType, expr).left;
+    }
+
     public RexRangeRef getSubQueryExpr(SqlCall call) {
       final SubQuery subQuery = getSubQuery(call);
       assert subQuery != null;

core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java

@@ -721,6 +721,10 @@ public RexBuilder getRexBuilder() {
         return rexBuilder;
       }
 
+      public SqlNode validateExpression(RelDataType rowType, SqlNode expr) {
+        throw new UnsupportedOperationException();
+      }
+
       public RexNode convertExpression(SqlNode e) {
         throw new UnsupportedOperationException();
       }

server/src/main/java/org/apache/calcite/sql/ddl/SqlCreateTable.java

@@ -207,7 +207,12 @@ public void execute(CalcitePrepare.Context context) {
               int iColumn, InitializerContext context) {
             final ColumnDef c = columns.get(iColumn);
             if (c.expr != null) {
-              return context.convertExpression(c.expr);
+              // REVIEW Danny 2019-10-09: Should we support validation for DDL nodes ?
+              final SqlNode validated = context.validateExpression(storedRowType, c.expr);
+              // The explicit specified type should have the same nullability
+              // with the column expression inferred type,
+              // actually they should be exactly the same.
+              return context.convertExpression(validated);
             }
             return super.newColumnDefaultValue(table, iColumn, context);
           }

server/src/test/java/org/apache/calcite/test/ServerTest.java

@@ -60,6 +60,7 @@ static Connection connect() throws SQLException {
                 SqlDdlParserImpl.class.getName() + "#FACTORY")
             .set(CalciteConnectionProperty.MATERIALIZATIONS_ENABLED,
                 "true")
+            .set(CalciteConnectionProperty.FUN, "standard,oracle")
             .build());
   }
 
@@ -418,6 +419,43 @@ static Connection connect() throws SQLException {
       }
     }
   }
+
+  @Test public void testVirtualColumnWithFunctions() throws Exception {
+    try (Connection c = connect();
+         Statement s = c.createStatement()) {
+      // Test builtin and library functions.
+      final String create = "create table t1 (\n"
+          + " h varchar(3) not null,\n"
+          + " i varchar(3),\n"
+          + " j int not null as (char_length(h)) virtual,\n"
+          + " k varchar(3) null as (trim(i)) virtual)";
+      boolean b = s.execute(create);
+      assertThat(b, is(false));
+
+      int x = s.executeUpdate("insert into t1 (h, i) values ('abc', 'de ')");
+      assertThat(x, is(1));
+
+      // In plan, "j" is replaced by "char_length(h)".
+      final String select = "select * from t1";
+      try (ResultSet r = s.executeQuery(select)) {
+        assertThat(r.next(), is(true));
+        assertThat(r.getString(1), is("abc"));
+        assertThat(r.getString(2), is("de "));
+        assertThat(r.getInt(3), is(3));
+        assertThat(r.getString(4), is("de"));
+        assertThat(r.next(), is(false));
+      }
+
+      final String plan = ""
+          + "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[CHAR_LENGTH($t0)], "
+          + "expr#3=[FLAG(BOTH)], expr#4=[' '], expr#5=[TRIM($t3, $t4, $t1)], proj#0..2=[{exprs}], K=[$t5])\n"
+          + "  EnumerableTableScan(table=[[T1]])\n";
+      try (ResultSet r = s.executeQuery("explain plan for " + select)) {
+        assertThat(r.next(), is(true));
+        assertThat(r.getString(1), isLinux(plan));
+      }
+    }
+  }
 }
 
 // End ServerTest.java