[CALCITE-5153] Create an immutable version of ListSqlOperatorTable

Create an immutable version of class ListSqlOperatorTable,
to be created via SqlOperatorTables.of.

class ListSqlOperatorTable still exists but its constructors
and add(SqlOperator) methods are deprecated. We hope that
people will create immutable operator tables most of the time.

Refactor MockSqlOperatorTable to be immutable. Methods that
mutate the table, such as addRamp, are replaced by methods
that return a copy.

When creating a ChainedSqlOperatorTable, flatten constituent
tables that are a ChainedSqlOperatorTable, and remove
constituent tables that are immutable and empty.

core/src/main/java/org/apache/calcite/prepare/CalciteCatalogReader.java

@@ -48,7 +48,7 @@
 import org.apache.calcite.sql.type.SqlReturnTypeInference;
 import org.apache.calcite.sql.type.SqlTypeFamily;
 import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.util.ListSqlOperatorTable;
+import org.apache.calcite.sql.util.SqlOperatorTables;
 import org.apache.calcite.sql.validate.SqlMoniker;
 import org.apache.calcite.sql.validate.SqlMonikerImpl;
 import org.apache.calcite.sql.validate.SqlMonikerType;
@@ -288,14 +288,14 @@ public static SqlOperatorTable operatorTable(String... classNames) {
           className, "*", true);
     }
 
-    final ListSqlOperatorTable table = new ListSqlOperatorTable();
+    final List<SqlOperator> list = new ArrayList<>();
     for (String name : schema.getFunctionNames()) {
       schema.getFunctions(name, true).forEach(function -> {
         final SqlIdentifier id = new SqlIdentifier(name, SqlParserPos.ZERO);
-        table.add(toOp(id, function));
+        list.add(toOp(id, function));
       });
     }
-    return table;
+    return SqlOperatorTables.of(list);
   }
 
   /** Converts a function to a {@link org.apache.calcite.sql.SqlOperator}. */

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

@@ -49,7 +49,7 @@ void lookupOperatorOverloads(SqlIdentifier opName,
 
   /**
    * Retrieves a list of all functions and operators in this table. Used for
-   * automated testing.
+   * automated testing. Depending on the table type, may or may not be mutable.
    *
    * @return list of SqlOperator objects
    */

core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperatorTableFactory.java

@@ -18,7 +18,6 @@
 
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlOperatorTable;
-import org.apache.calcite.sql.util.ListSqlOperatorTable;
 import org.apache.calcite.sql.util.SqlOperatorTables;
 import org.apache.calcite.util.Util;
 
@@ -29,6 +28,8 @@
 import com.google.common.collect.ImmutableSet;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 
@@ -77,7 +78,7 @@ private SqlLibraryOperatorTableFactory(Class... classes) {
   /** Creates an operator table that contains operators in the given set of
    * libraries. */
   private SqlOperatorTable create(ImmutableSet<SqlLibrary> librarySet) {
-    final ImmutableList.Builder<SqlOperator> list = ImmutableList.builder();
+    final List<SqlOperator> list = new ArrayList<>();
     boolean custom = false;
     boolean standard = false;
     for (SqlLibrary library : librarySet) {
@@ -112,7 +113,7 @@ private SqlOperatorTable create(ImmutableSet<SqlLibrary> librarySet) {
         }
       }
     }
-    SqlOperatorTable operatorTable = new ListSqlOperatorTable(list.build());
+    SqlOperatorTable operatorTable = SqlOperatorTables.of(list);
     if (standard) {
       operatorTable =
           SqlOperatorTables.chain(SqlStdOperatorTable.instance(),

core/src/main/java/org/apache/calcite/sql/util/ListSqlOperatorTable.java

@@ -40,16 +40,36 @@ public class ListSqlOperatorTable implements SqlOperatorTable {
 
   //~ Constructors -----------------------------------------------------------
 
+  /** Creates an empty, mutable ListSqlOperatorTable.
+   *
+   * @deprecated Use {@link SqlOperatorTables#of}, which creates an immutable
+   * table. */
+  @Deprecated // to be removed before 2.0
   public ListSqlOperatorTable() {
-    this(new ArrayList<>());
+    this(new ArrayList<>(), false);
   }
 
+  /** Creates a mutable ListSqlOperatorTable backed by a given list.
+   *
+   * @deprecated Use {@link SqlOperatorTables#of}, which creates an immutable
+   * table. */
+  @Deprecated // to be removed before 2.0
   public ListSqlOperatorTable(List<SqlOperator> operatorList) {
+    this(operatorList, false);
+  }
+
+  // internal constructor
+  ListSqlOperatorTable(List<SqlOperator> operatorList, boolean ignored) {
     this.operatorList = operatorList;
   }
 
   //~ Methods ----------------------------------------------------------------
 
+  /** Adds an operator to this table.
+   *
+   * @deprecated Use {@link SqlOperatorTables#of}, which creates an immutable
+   * table. */
+  @Deprecated // to be removed before 2.0
   public void add(SqlOperator op) {
     operatorList.add(op);
   }

core/src/main/java/org/apache/calcite/sql/util/SqlOperatorTables.java

@@ -18,19 +18,24 @@
 
 import org.apache.calcite.prepare.CalciteCatalogReader;
 import org.apache.calcite.runtime.GeoFunctions;
+import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlOperatorTable;
 import org.apache.calcite.sql.fun.SqlGeoFunctions;
 
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.function.Supplier;
 
 /**
  * Utilities for {@link SqlOperatorTable}s.
  */
-public class SqlOperatorTables extends ReflectiveSqlOperatorTable {
+public class SqlOperatorTables {
+  private SqlOperatorTables() {}
 
+  @SuppressWarnings("FunctionalExpressionCanBeFolded")
   private static final Supplier<SqlOperatorTable> SPATIAL =
       Suppliers.memoize(SqlOperatorTables::createSpatial)::get;
 
@@ -47,16 +52,54 @@ public static SqlOperatorTable spatialInstance() {
 
   /** Creates a composite operator table. */
   public static SqlOperatorTable chain(Iterable<SqlOperatorTable> tables) {
-    final ImmutableList<SqlOperatorTable> list =
-        ImmutableList.copyOf(tables);
+    final List<SqlOperatorTable> list = new ArrayList<>();
+    for (SqlOperatorTable table : tables) {
+      addFlattened(list, table);
+    }
     if (list.size() == 1) {
       return list.get(0);
     }
-    return new ChainedSqlOperatorTable(list);
+    return new ChainedSqlOperatorTable(ImmutableList.copyOf(list));
+  }
+
+  @SuppressWarnings("StatementWithEmptyBody")
+  private static void addFlattened(List<SqlOperatorTable> list,
+      SqlOperatorTable table) {
+    if (table instanceof ChainedSqlOperatorTable) {
+      ChainedSqlOperatorTable chainedTable = (ChainedSqlOperatorTable) table;
+      for (SqlOperatorTable table2 : chainedTable.tableList) {
+        addFlattened(list, table2);
+      }
+    } else if (table instanceof ImmutableListSqlOperatorTable
+        && table.getOperatorList().isEmpty()) {
+      // Table is empty and will remain empty; don't add it.
+    } else {
+      list.add(table);
+    }
   }
 
   /** Creates a composite operator table from an array of tables. */
   public static SqlOperatorTable chain(SqlOperatorTable... tables) {
     return chain(ImmutableList.copyOf(tables));
   }
+
+  /** Creates an operator table that contains an immutable list of operators. */
+  public static SqlOperatorTable of(Iterable<? extends SqlOperator> list) {
+    return new ImmutableListSqlOperatorTable(ImmutableList.copyOf(list));
+  }
+
+  /** Creates an operator table that contains the given operator or
+   * operators. */
+  public static SqlOperatorTable of(SqlOperator... operators) {
+    return of(ImmutableList.copyOf(operators));
+  }
+
+  /** Subclass of {@link ListSqlOperatorTable} that is immutable.
+   * Operators cannot be added or removed after creation. */
+  private static class ImmutableListSqlOperatorTable
+      extends ListSqlOperatorTable {
+    ImmutableListSqlOperatorTable(ImmutableList<SqlOperator> operatorList) {
+      super(operatorList, false);
+    }
+  }
 }

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

@@ -62,12 +62,10 @@
 import org.apache.calcite.sql.dialect.OracleSqlDialect;
 import org.apache.calcite.sql.dialect.PostgresqlSqlDialect;
 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.SqlParser;
 import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
 import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.util.SqlOperatorTables;
 import org.apache.calcite.sql.util.SqlShuttle;
 import org.apache.calcite.sql.validate.SqlConformance;
 import org.apache.calcite.sql.validate.SqlConformanceEnum;
@@ -141,19 +139,15 @@ private static Planner getPlanner(List<RelTraitDef> traitDefs,
       SqlParser.Config parserConfig, SchemaPlus schema,
       SqlToRelConverter.Config sqlToRelConf, Collection<SqlLibrary> librarySet,
       RelDataTypeSystem typeSystem, Program... programs) {
-    final MockSqlOperatorTable operatorTable =
-        new MockSqlOperatorTable(
-            SqlOperatorTables.chain(SqlStdOperatorTable.instance(),
-                SqlLibraryOperatorTableFactory.INSTANCE
-                    .getOperatorTable(librarySet)));
-    MockSqlOperatorTable.addRamp(operatorTable);
     final FrameworkConfig config = Frameworks.newConfigBuilder()
         .parserConfig(parserConfig)
         .defaultSchema(schema)
         .traitDefs(traitDefs)
         .sqlToRelConverterConfig(sqlToRelConf)
         .programs(programs)
-        .operatorTable(operatorTable)
+        .operatorTable(MockSqlOperatorTable.standard()
+            .plus(librarySet)
+            .extend())
         .typeSystem(typeSystem)
         .build();
     return Frameworks.getPlanner(config);

core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java

@@ -9166,9 +9166,8 @@ public void _testGroupExpressionEquivalenceParams() {
 
   /** Tests using case-insensitive matching of user-defined functions. */
   @Test void testCaseInsensitiveUdfs() {
-    final MockSqlOperatorTable operatorTable =
-        new MockSqlOperatorTable(SqlStdOperatorTable.instance());
-    MockSqlOperatorTable.addRamp(operatorTable);
+    final SqlOperatorTable operatorTable =
+        MockSqlOperatorTable.standard().extend();
     final SqlValidatorFixture insensitive = fixture()
         .withCaseSensitive(false)
         .withQuoting(Quoting.BRACKET)
@@ -12135,9 +12134,8 @@ private void checkCustomColumnResolving(String table) {
   }
 
   @Test void testInvalidFunctionCall() {
-    final MockSqlOperatorTable operatorTable =
-        new MockSqlOperatorTable(SqlStdOperatorTable.instance());
-    MockSqlOperatorTable.addRamp(operatorTable);
+    final SqlOperatorTable operatorTable =
+        MockSqlOperatorTable.standard().extend();
 
     // With implicit type coercion.
     expr("^unknown_udf(1, 2)^")
@@ -12257,9 +12255,8 @@ private void checkCustomColumnResolving(String table) {
         .withExtendedCatalog()
         .type("RecordType(BIGINT EXPR$0) NOT NULL");
 
-    final MockSqlOperatorTable operatorTable =
-        new MockSqlOperatorTable(SqlStdOperatorTable.instance());
-    MockSqlOperatorTable.addRamp(operatorTable);
+    final SqlOperatorTable operatorTable =
+        MockSqlOperatorTable.standard().extend();
     sql("select * FROM TABLE(ROW_FUNC()) AS T(a, b)")
         .withOperatorTable(operatorTable)
         .type("RecordType(BIGINT NOT NULL A, BIGINT B) NOT NULL");

core/src/test/java/org/apache/calcite/tools/PlannerTest.java

@@ -65,15 +65,13 @@
 import org.apache.calcite.sql.SqlFunctionCategory;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
-import org.apache.calcite.sql.SqlOperatorTable;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.test.SqlTests;
 import org.apache.calcite.sql.type.OperandTypes;
 import org.apache.calcite.sql.type.ReturnTypes;
 import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.util.ListSqlOperatorTable;
 import org.apache.calcite.sql.util.SqlOperatorTables;
 import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql.validate.SqlValidatorScope;
@@ -206,16 +204,13 @@ private String toString(RelNode rel) {
   }
 
   @Test void testValidateUserDefinedAggregate() throws Exception {
-    final SqlStdOperatorTable stdOpTab = SqlStdOperatorTable.instance();
-    SqlOperatorTable opTab =
-        SqlOperatorTables.chain(stdOpTab,
-            new ListSqlOperatorTable(
-                ImmutableList.of(new MyCountAggFunction())));
     final SchemaPlus rootSchema = Frameworks.createRootSchema(true);
     final FrameworkConfig config = Frameworks.newConfigBuilder()
         .defaultSchema(
             CalciteAssert.addSchema(rootSchema, CalciteAssert.SchemaSpec.HR))
-        .operatorTable(opTab)
+        .operatorTable(
+            SqlOperatorTables.chain(SqlStdOperatorTable.instance(),
+                SqlOperatorTables.of(new MyCountAggFunction())))
         .build();
     final Planner planner = Frameworks.getPlanner(config);
     SqlNode parse =

testkit/src/main/java/org/apache/calcite/sql/test/SqlTestFactory.java

@@ -76,11 +76,7 @@ public class SqlTestFactory {
           SqlValidator.Config.DEFAULT,
           SqlToRelConverter.CONFIG,
           SqlStdOperatorTable.instance())
-      .withOperatorTable(o -> {
-        MockSqlOperatorTable opTab = new MockSqlOperatorTable(o);
-        MockSqlOperatorTable.addRamp(opTab);
-        return opTab;
-      });
+      .withOperatorTable(o -> MockSqlOperatorTable.of(o).extend());
 
   public final ConnectionFactory connectionFactory;
   public final TypeFactoryFactory typeFactoryFactory;