[CALCITE-2670] Combine similar JSON aggregate functions in operator t…

…able Convert parameters from Enum to more specific types; include varying flag values in JSON aggregate functions. Some cosmetic stuff. Close #916
apache · Dec 6, 2018 · f3655e1 · f3655e1
1 parent 40d12b7
commit f3655e1
Show file tree

Hide file tree

Showing 21 changed files with 304 additions and 124 deletions.
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
@@ -5267,14 +5267,8 @@ SqlCall JsonObjectAggFunctionCall() :
         }
     ]
     <RPAREN> {
-        switch (nullClause) {
-        case ABSENT_ON_NULL:
-          return SqlStdOperatorTable.JSON_OBJECTAGG_ABSENT_ON_NULL.createCall(span.end(this), args);
-        case NULL_ON_NULL:
-          return SqlStdOperatorTable.JSON_OBJECTAGG_NULL_ON_NULL.createCall(span.end(this), args);
-        default:
-          return SqlStdOperatorTable.JSON_OBJECTAGG_NULL_ON_NULL.createCall(span.end(this), args);
-        }
+        return SqlStdOperatorTable.JSON_OBJECTAGG.with(nullClause)
+            .createCall(span.end(this), args);
     }
 }
 
@@ -5313,7 +5307,7 @@ SqlCall JsonArrayFunctionCall() :
 
 SqlCall JsonArrayAggFunctionCall() :
 {
-    final SqlNode arg;
+    final SqlNode[] args = new SqlNode[1];
     List<SqlNode> list;
     final Span span;
     SqlJsonConstructorNullClause nullClause =
@@ -5322,21 +5316,17 @@ SqlCall JsonArrayAggFunctionCall() :
 }
 {
     <JSON_ARRAYAGG> { span = span(); }
-    <LPAREN> arg = JsonValueExpression(false)
+    <LPAREN> e = JsonValueExpression(false) {
+        args[0] = e;
+    }
     [
         e = JsonConstructorNullClause() {
             nullClause = (SqlJsonConstructorNullClause) ((SqlLiteral) e).getValue();
         }
     ]
     <RPAREN> {
-        switch (nullClause) {
-        case ABSENT_ON_NULL:
-          return SqlStdOperatorTable.JSON_ARRAYAGG_ABSENT_ON_NULL.createCall(span.end(this), arg);
-        case NULL_ON_NULL:
-          return SqlStdOperatorTable.JSON_ARRAYAGG_NULL_ON_NULL.createCall(span.end(this), arg);
-        default:
-          return SqlStdOperatorTable.JSON_ARRAYAGG_ABSENT_ON_NULL.createCall(span.end(this), arg);
-        }
+        return SqlStdOperatorTable.JSON_ARRAYAGG.with(nullClause)
+            .createCall(span.end(this), args);
     }
 }
 

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -45,6 +45,8 @@
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlBinaryOperator;
 import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlJsonArrayAggAggFunction;
+import org.apache.calcite.sql.fun.SqlJsonObjectAggAggFunction;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.sql.fun.SqlTrimFunction;
 import org.apache.calcite.sql.type.SqlTypeName;
@@ -157,12 +159,10 @@
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.ITEM;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_API_COMMON_SYNTAX;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_ARRAY;
-import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_ARRAYAGG_ABSENT_ON_NULL;
-import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_ARRAYAGG_NULL_ON_NULL;
+import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_ARRAYAGG;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_EXISTS;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECT;
-import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECTAGG_ABSENT_ON_NULL;
-import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECTAGG_NULL_ON_NULL;
+import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECTAGG;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_QUERY;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_STRUCTURED_VALUE_EXPRESSION;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_VALUE_ANY;
@@ -448,19 +448,13 @@ public Expression implement(RexToLixTranslator translator,
     defineMethod(JSON_VALUE_ANY, BuiltInMethod.JSON_VALUE_ANY.method, NullPolicy.NONE);
     defineMethod(JSON_QUERY, BuiltInMethod.JSON_QUERY.method, NullPolicy.NONE);
     defineMethod(JSON_OBJECT, BuiltInMethod.JSON_OBJECT.method, NullPolicy.NONE);
-    aggMap.put(JSON_OBJECTAGG_NULL_ON_NULL,
+    aggMap.put(JSON_OBJECTAGG,
         JsonObjectAggImplementor
-            .supplierFor(BuiltInMethod.JSON_OBJECTAGG_ADD_NULL_ON_NULL.method));
-    aggMap.put(JSON_OBJECTAGG_ABSENT_ON_NULL,
-        JsonObjectAggImplementor
-            .supplierFor(BuiltInMethod.JSON_OBJECTAGG_ADD_ABSENT_ON_NULL.method));
+            .supplierFor(BuiltInMethod.JSON_OBJECTAGG_ADD.method));
     defineMethod(JSON_ARRAY, BuiltInMethod.JSON_ARRAY.method, NullPolicy.NONE);
-    aggMap.put(JSON_ARRAYAGG_NULL_ON_NULL,
-        JsonArrayAggImplementor
-            .supplierFor(BuiltInMethod.JSON_ARRAYAGG_ADD_NULL_ON_NULL.method));
-    aggMap.put(JSON_ARRAYAGG_ABSENT_ON_NULL,
+    aggMap.put(JSON_ARRAYAGG,
         JsonArrayAggImplementor
-            .supplierFor(BuiltInMethod.JSON_ARRAYAGG_ADD_ABSENT_ON_NULL.method));
+            .supplierFor(BuiltInMethod.JSON_ARRAYAGG_ADD.method));
     defineImplementor(IS_JSON_VALUE, NullPolicy.NONE,
             new MethodImplementor(BuiltInMethod.IS_JSON_VALUE.method), false);
     defineImplementor(IS_JSON_OBJECT, NullPolicy.NONE,
@@ -1813,12 +1807,16 @@ static Supplier<JsonObjectAggImplementor> supplierFor(Method m) {
     }
 
     @Override public void implementAdd(AggContext info, AggAddContext add) {
+      final SqlJsonObjectAggAggFunction function =
+          (SqlJsonObjectAggAggFunction) info.aggregation();
       add.currentBlock().add(
           Expressions.statement(
               Expressions.call(m,
                   Iterables.concat(
                       Collections.singletonList(add.accumulator().get(0)),
-                      add.arguments()))));
+                      add.arguments(),
+                      Collections.singletonList(
+                          Expressions.constant(function.getNullClause()))))));
     }
 
     @Override public Expression implementResult(AggContext info,
@@ -1854,12 +1852,16 @@ static Supplier<JsonArrayAggImplementor> supplierFor(Method m) {
 
     @Override public void implementAdd(AggContext info,
         AggAddContext add) {
+      final SqlJsonArrayAggAggFunction function =
+          (SqlJsonArrayAggAggFunction) info.aggregation();
       add.currentBlock().add(
           Expressions.statement(
               Expressions.call(m,
                   Iterables.concat(
                       Collections.singletonList(add.accumulator().get(0)),
-                      add.arguments()))));
+                      add.arguments(),
+                      Collections.singletonList(
+                          Expressions.constant(function.getNullClause()))))));
     }
 
     @Override public Expression implementResult(AggContext info,

diff --git a/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java b/core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java
@@ -213,6 +213,8 @@ public Type getJavaClass(RelDataType type) {
         return ByteString.class;
       case GEOMETRY:
         return GeoFunctions.Geom.class;
+      case SYMBOL:
+        return Enum.class;
       case ANY:
         return Object.class;
       }

diff --git a/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java b/core/src/main/java/org/apache/calcite/rel/core/AggregateCall.java
@@ -254,7 +254,7 @@ public AggregateCall rename(String name) {
   }
 
   public String toString() {
-    StringBuilder buf = new StringBuilder(aggFunction.getName());
+    StringBuilder buf = new StringBuilder(aggFunction.toString());
     buf.append("(");
     if (distinct) {
       buf.append((argList.size() == 0) ? "DISTINCT" : "DISTINCT ");

diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -2409,7 +2409,7 @@ public static PathContext jsonApiCommonSyntax(Object input, String pathSpec) {
       if (!matcher.matches()) {
         throw RESOURCE.illegalJsonPathSpec(pathSpec).ex();
       }
-      PathMode mode = PathMode.valueOf(matcher.group(1).toUpperCase(Locale.ENGLISH));
+      PathMode mode = PathMode.valueOf(matcher.group(1).toUpperCase(Locale.ROOT));
       String pathWff = matcher.group(2);
       DocumentContext ctx;
       switch (mode) {
@@ -2636,14 +2636,6 @@ public static void jsonObjectAggAdd(Map map, String k, Object v,
     }
   }
 
-  public static void jsonObjectAggAddNullOnNull(Map map, String k, Object v) {
-    jsonObjectAggAdd(map, k, v, SqlJsonConstructorNullClause.NULL_ON_NULL);
-  }
-
-  public static void jsonObjectAggAddAbsentOnNull(Map map, String k, Object v) {
-    jsonObjectAggAdd(map, k, v, SqlJsonConstructorNullClause.ABSENT_ON_NULL);
-  }
-
   public static String jsonArray(SqlJsonConstructorNullClause nullClause,
       Object... elements) {
     List<Object> list = new ArrayList<>();
@@ -2670,14 +2662,6 @@ public static void jsonArrayAggAdd(List list, Object element,
     }
   }
 
-  public static void jsonArrayAggAddNullOnNull(List list, Object element) {
-    jsonArrayAggAdd(list, element, SqlJsonConstructorNullClause.NULL_ON_NULL);
-  }
-
-  public static void jsonArrayAggAddAbsentOnNull(List list, Object element) {
-    jsonArrayAggAdd(list, element, SqlJsonConstructorNullClause.ABSENT_ON_NULL);
-  }
-
   public static boolean isJsonValue(String input) {
     try {
       dejsonize(input);

diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJsonConstructorNullClause.java b/core/src/main/java/org/apache/calcite/sql/SqlJsonConstructorNullClause.java
@@ -20,7 +20,14 @@
  * Indicating that how do Json constructors handle null
  */
 public enum SqlJsonConstructorNullClause {
-  NULL_ON_NULL, ABSENT_ON_NULL
+  NULL_ON_NULL("NULL ON NULL"),
+  ABSENT_ON_NULL("ABSENT ON NULL");
+
+  public final String sql;
+
+  SqlJsonConstructorNullClause(String sql) {
+    this.sql = sql;
+  }
 }
 
 // End SqlJsonConstructorNullClause.java
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJsonEmptyOrError.java b/core/src/main/java/org/apache/calcite/sql/SqlJsonEmptyOrError.java
@@ -23,10 +23,11 @@
  * EmptyOrErrorBehavior is invoked.
  */
 public enum SqlJsonEmptyOrError {
-  EMPTY, ERROR;
+  EMPTY,
+  ERROR;
 
   @Override public String toString() {
-    return String.format(Locale.ENGLISH, "SqlJsonEmptyOrError[%s]", name());
+    return String.format(Locale.ROOT, "SqlJsonEmptyOrError[%s]", name());
   }
 }
 

diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJsonExistsErrorBehavior.java b/core/src/main/java/org/apache/calcite/sql/SqlJsonExistsErrorBehavior.java
@@ -20,7 +20,10 @@
  * Categorizing Json exists error behaviors.
  */
 public enum SqlJsonExistsErrorBehavior {
-  TRUE, FALSE, UNKNOWN, ERROR
+  TRUE,
+  FALSE,
+  UNKNOWN,
+  ERROR
 }
 
 // End SqlJsonExistsErrorBehavior.java
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJsonQueryEmptyOrErrorBehavior.java b/core/src/main/java/org/apache/calcite/sql/SqlJsonQueryEmptyOrErrorBehavior.java
@@ -28,7 +28,7 @@ public enum SqlJsonQueryEmptyOrErrorBehavior {
   EMPTY_OBJECT;
 
   @Override public String toString() {
-    return String.format(Locale.ENGLISH,
+    return String.format(Locale.ROOT,
         "SqlJsonQueryEmptyOrErrorBehavior[%s]", name());
   }
 }

diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJsonValueEmptyOrErrorBehavior.java b/core/src/main/java/org/apache/calcite/sql/SqlJsonValueEmptyOrErrorBehavior.java
@@ -27,7 +27,7 @@ public enum SqlJsonValueEmptyOrErrorBehavior {
   DEFAULT;
 
   @Override public String toString() {
-    return String.format(Locale.ENGLISH,
+    return String.format(Locale.ROOT,
         "SqlJsonValueEmptyOrErrorBehavior[%s]", name());
   }
 }

diff --git a/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java b/core/src/main/java/org/apache/calcite/sql/dialect/MssqlSqlDialect.java
@@ -180,8 +180,8 @@ private void unparseFloor(SqlWriter writer, SqlCall call) {
 
   private void unparseSqlIntervalLiteralMssql(
       SqlWriter writer, SqlIntervalLiteral literal, int sign) {
-    SqlIntervalLiteral.IntervalValue interval
-        = (SqlIntervalLiteral.IntervalValue) literal.getValue();
+    final SqlIntervalLiteral.IntervalValue interval =
+        (SqlIntervalLiteral.IntervalValue) literal.getValue();
     unparseSqlIntervalQualifier(writer, interval.getIntervalQualifier(),
         RelDataTypeSystem.DEFAULT);
     writer.sep(",", true);

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java
@@ -16,52 +16,69 @@
  */
 package org.apache.calcite.sql.fun;
 
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.sql.SqlAggFunction;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlFunctionCategory;
 import org.apache.calcite.sql.SqlJsonConstructorNullClause;
 import org.apache.calcite.sql.SqlKind;
-import org.apache.calcite.sql.SqlLiteral;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.SqlWriter;
 import org.apache.calcite.sql.type.OperandTypes;
 import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorImpl;
+import org.apache.calcite.sql.validate.SqlValidatorScope;
 import org.apache.calcite.util.Optionality;
 
+import java.util.Locale;
+import java.util.Objects;
+
 /**
- * The <code>JSON_OBJECTAGG</code> aggregation function.
+ * The <code>JSON_OBJECTAGG</code> aggregate function.
  */
 public class SqlJsonArrayAggAggFunction extends SqlAggFunction {
   private final SqlJsonConstructorNullClause nullClause;
 
   public SqlJsonArrayAggAggFunction(String name,
       SqlJsonConstructorNullClause nullClause) {
     super(name, null, SqlKind.JSON_ARRAYAGG, ReturnTypes.VARCHAR_2000, null,
-        OperandTypes.ANY, SqlFunctionCategory.SYSTEM, false, false,
-        Optionality.FORBIDDEN);
-    this.nullClause = nullClause;
+        OperandTypes.family(SqlTypeFamily.ANY), SqlFunctionCategory.SYSTEM,
+        false, false, Optionality.FORBIDDEN);
+    this.nullClause = Objects.requireNonNull(nullClause);
   }
 
   @Override public void unparse(SqlWriter writer, SqlCall call, int leftPrec,
       int rightPrec) {
     assert call.operandCount() == 1;
     final SqlWriter.Frame frame = writer.startFunCall("JSON_ARRAYAGG");
     call.operand(0).unparse(writer, leftPrec, rightPrec);
-    switch (nullClause) {
-    case ABSENT_ON_NULL:
-      writer.keyword("ABSENT ON NULL");
-      break;
-    case NULL_ON_NULL:
-      writer.keyword("NULL ON NULL");
-      break;
-    default:
-      throw new IllegalStateException("unreachable code");
-    }
+    writer.keyword(nullClause.sql);
     writer.endFunCall(frame);
   }
 
-  private <E extends Enum<E>> E getEnumValue(SqlNode operand) {
-    return (E) ((SqlLiteral) operand).getValue();
+  @Override public RelDataType deriveType(SqlValidator validator,
+      SqlValidatorScope scope, SqlCall call) {
+    // To prevent operator rewriting by SqlFunction#deriveType.
+    for (SqlNode operand : call.getOperandList()) {
+      RelDataType nodeType = validator.deriveType(scope, operand);
+      ((SqlValidatorImpl) validator).setValidatedNodeType(operand, nodeType);
+    }
+    return validateOperands(validator, scope, call);
+  }
+
+  @Override public String toString() {
+    return getName() + String.format(Locale.ROOT, "<%s>", nullClause);
+  }
+
+  public SqlJsonArrayAggAggFunction with(SqlJsonConstructorNullClause nullClause) {
+    return this.nullClause == nullClause ? this
+        : new SqlJsonArrayAggAggFunction(getName(), nullClause);
+  }
+
+  public SqlJsonConstructorNullClause getNullClause() {
+    return nullClause;
   }
 }
 

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayFunction.java
@@ -67,7 +67,7 @@ public SqlJsonArrayFunction() {
     final StringBuilder sb = new StringBuilder();
     sb.append("{0}(");
     for (int i = 1; i < operandsCount; i++) {
-      sb.append(String.format(Locale.ENGLISH, "{%d} ", i + 1));
+      sb.append(String.format(Locale.ROOT, "{%d} ", i + 1));
     }
     sb.append("{1})");
     return sb.toString();