[CALCITE-5747] Conflicting FLOOR return type between Calcite and BigQ…

…uery
looker-open-source · Sep 8, 2023 · 6175efe · 6175efe
1 parent 3a3f3a2
commit 6175efe
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 6 deletions.
diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj
@@ -114,6 +114,7 @@ import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.parser.SqlParserUtil;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlConformanceEnum;
 import org.apache.calcite.util.Glossary;
 import org.apache.calcite.util.Pair;
 import org.apache.calcite.util.SourceStringReader;
@@ -7347,9 +7348,7 @@ SqlNode StandardFloorCeilOptions(Span s, boolean floorFlag) :
         }
     )?
     <RPAREN> {
-        SqlOperator op = floorFlag
-            ? SqlStdOperatorTable.FLOOR
-            : SqlStdOperatorTable.CEIL;
+        SqlOperator op = SqlStdOperatorTable.floorCeil(floorFlag, (SqlConformanceEnum) this.conformance);
         function =  op.createCall(s.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
@@ -141,6 +141,7 @@
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.ATANH;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.BOOL_AND;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.BOOL_OR;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.CEIL_BIG_QUERY;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHAR;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.COMPRESS;
@@ -163,6 +164,7 @@
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.EXISTS_NODE;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.EXTRACT_VALUE;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.EXTRACT_XML;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.FLOOR_BIG_QUERY;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.FORMAT_DATE;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.FORMAT_DATETIME;
 import static org.apache.calcite.sql.fun.SqlLibraryOperators.FORMAT_TIME;
@@ -618,6 +620,9 @@ Builder populate2() {
       map.put(TIMESTAMP_TRUNC, map.get(FLOOR));
       map.put(TIME_TRUNC, map.get(FLOOR));
       map.put(DATETIME_TRUNC, map.get(FLOOR));
+      // BigQuery FLOOR and CEIL should use same implementation as standard
+      map.put(CEIL_BIG_QUERY, map.get(CEIL));
+      map.put(FLOOR_BIG_QUERY, map.get(FLOOR));
 
       map.put(LAST_DAY,
           new LastDayImplementor("lastDay", BuiltInMethod.LAST_DAY));

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlFloorFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlFloorFunction.java
@@ -16,6 +16,7 @@
  */
 package org.apache.calcite.sql.fun;
 
+import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlFunction;
 import org.apache.calcite.sql.SqlFunctionCategory;
@@ -29,18 +30,29 @@
 import org.apache.calcite.sql.parser.SqlParserPos;
 import org.apache.calcite.sql.type.OperandTypes;
 import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlOperandTypeChecker;
+import org.apache.calcite.sql.type.SqlOperandTypeInference;
+import org.apache.calcite.sql.type.SqlReturnTypeInference;
 import org.apache.calcite.sql.validate.SqlMonotonicity;
 import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql.validate.SqlValidatorScope;
 
 import com.google.common.base.Preconditions;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * Definition of the "FLOOR" and "CEIL" built-in SQL functions.
  */
 public class SqlFloorFunction extends SqlMonotonicUnaryFunction {
   //~ Constructors -----------------------------------------------------------
-
+  private SqlFloorFunction(String name, SqlKind kind,
+      @Nullable SqlReturnTypeInference returnTypeInference,
+      @Nullable SqlOperandTypeInference operandTypeInference,
+      @Nullable SqlOperandTypeChecker operandTypeChecker,
+      SqlFunctionCategory funcType) {
+    super(name, kind, returnTypeInference, operandTypeInference, operandTypeChecker, funcType);
+  }
   public SqlFloorFunction(SqlKind kind) {
     super(kind.name(), kind, ReturnTypes.ARG0_OR_EXACT_NO_SCALE, null,
         OperandTypes.NUMERIC_OR_INTERVAL.or(
@@ -53,6 +65,16 @@ public SqlFloorFunction(SqlKind kind) {
     Preconditions.checkArgument(kind == SqlKind.FLOOR || kind == SqlKind.CEIL);
   }
 
+  public SqlFloorFunction withName(String name) {
+    return new SqlFloorFunction(name, getKind(), getReturnTypeInference(),
+        getOperandTypeInference(), getOperandTypeChecker(), getFunctionType());
+  }
+
+  public SqlFloorFunction withReturnTypeInference(SqlReturnTypeInference returnTypeInference) {
+    return new SqlFloorFunction(getName(), getKind(), returnTypeInference,
+        getOperandTypeInference(), getOperandTypeChecker(), getFunctionType());
+  }
+
   //~ Methods ----------------------------------------------------------------
 
   @Override public SqlMonotonicity getMonotonicity(SqlOperatorBinding call) {
@@ -74,6 +96,16 @@ public SqlFloorFunction(SqlKind kind) {
     writer.endFunCall(frame);
   }
 
+  @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);
+      validator.setValidatedNodeType(operand, nodeType);
+    }
+    return validateOperands(validator, scope, call);
+  }
+
   @Override public void validateCall(SqlCall call, SqlValidator validator,
       SqlValidatorScope scope, SqlValidatorScope operandScope) {
     super.validateCall(call, validator, scope, operandScope);
@@ -93,6 +125,10 @@ public SqlFloorFunction(SqlKind kind) {
     }
   }
 
+  @Override public String getName() {
+    return kind.name();
+  }
+
   /**
    * Copies a {@link SqlCall}, replacing the time unit operand with the given
    * literal.

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -405,6 +405,20 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
           ReturnTypes.LEAST_RESTRICTIVE.andThen(SqlTypeTransforms.TO_NULLABLE),
           OperandTypes.SAME_VARIADIC);
 
+  /** The "CEIL(value)" function. Identical to the standard <code>CEIL</code> function
+   * except the return type should be a double if the operand is an integer. */
+  @LibraryOperator(libraries = {BIG_QUERY})
+  public static final SqlFunction CEIL_BIG_QUERY = new SqlFloorFunction(SqlKind.CEIL)
+      .withName("CEIL_BIG_QUERY")
+      .withReturnTypeInference(ReturnTypes.ARG0_EXCEPT_INTEGER_NULLABLE);
+
+  /** The "FLOOR(value)" function. Identical to the stadnard <code>FLOOR</code> function
+   * except the return type should be a double if the operand is an integer. */
+  @LibraryOperator(libraries = {BIG_QUERY})
+  public static final SqlFunction FLOOR_BIG_QUERY = new SqlFloorFunction(SqlKind.FLOOR)
+      .withName("FLOOR_BIG_QUERY")
+      .withReturnTypeInference(ReturnTypes.ARG0_EXCEPT_INTEGER_NULLABLE);
+
   /**
    * The <code>TRANSLATE(<i>string_expr</i>, <i>search_chars</i>,
    * <i>replacement_chars</i>)</code> function returns <i>string_expr</i> with

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlMonotonicUnaryFunction.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlMonotonicUnaryFunction.java
@@ -37,9 +37,9 @@ public class SqlMonotonicUnaryFunction extends SqlFunction {
   protected SqlMonotonicUnaryFunction(
       String name,
       SqlKind kind,
-      SqlReturnTypeInference returnTypeInference,
+      @Nullable SqlReturnTypeInference returnTypeInference,
       @Nullable SqlOperandTypeInference operandTypeInference,
-      SqlOperandTypeChecker operandTypeChecker,
+      @Nullable SqlOperandTypeChecker operandTypeChecker,
       SqlFunctionCategory funcType) {
     super(
         name,

diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -68,6 +68,7 @@
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.util.ReflectiveSqlOperatorTable;
 import org.apache.calcite.sql.validate.SqlConformance;
+import org.apache.calcite.sql.validate.SqlConformanceEnum;
 import org.apache.calcite.sql.validate.SqlModality;
 import org.apache.calcite.sql2rel.AuxiliaryConverter;
 import org.apache.calcite.util.Litmus;
@@ -2702,4 +2703,14 @@ public static SqlOperator like(boolean negated, boolean caseSensitive) {
     }
   }
 
+  /** Returns the operator for {@code FLOOR} and {@code CEIL} with given floor flag
+   * and library. */
+  public static SqlOperator floorCeil(boolean floor, SqlConformanceEnum conformance) {
+    switch (conformance) {
+    case BIG_QUERY:
+      return floor ? SqlLibraryOperators.FLOOR_BIG_QUERY : SqlLibraryOperators.CEIL_BIG_QUERY;
+    default:
+      return floor ? SqlStdOperatorTable.FLOOR : SqlStdOperatorTable.CEIL;
+    }
+  }
 }
diff --git a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
@@ -528,6 +528,29 @@ public static SqlCall stripSeparator(SqlCall call) {
       opBinding -> opBinding.getTypeFactory().leastRestrictive(
           opBinding.collectOperandTypes());
 
+  /**
+   * Type-inference strategy that returns the type of the first operand, unless it
+   * is an integer type, in which case the return type is DOUBLE.
+   */
+  public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER = opBinding ->  {
+    RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+    SqlTypeName op = opBinding.getOperandType(0).getSqlTypeName();
+    if (SqlTypeName.INT_TYPES.contains(op)) {
+      return typeFactory.createTypeWithNullability(
+          typeFactory.createSqlType(SqlTypeName.DOUBLE), true);
+    } else {
+      return typeFactory.createTypeWithNullability(typeFactory.createSqlType(op), true);
+    }
+  };
+
+  /**
+   * Same as {@link #ARG0_EXCEPT_INTEGER} but returns with nullability if any of
+   * the operands is nullable by using
+   * {@link org.apache.calcite.sql.type.SqlTypeTransforms#TO_NULLABLE}.
+   */
+  public static final SqlReturnTypeInference ARG0_EXCEPT_INTEGER_NULLABLE =
+      ARG0_EXCEPT_INTEGER.andThen(SqlTypeTransforms.TO_NULLABLE);
+
   /**
    * Returns the same type as the multiset carries. The multiset type returned
    * is the least restrictive of the call's multiset operands

diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -54,6 +54,7 @@
 import org.apache.calcite.sql.SqlWriter;
 import org.apache.calcite.sql.SqlWriterConfig;
 import org.apache.calcite.sql.dialect.AnsiSqlDialect;
+import org.apache.calcite.sql.dialect.BigQuerySqlDialect;
 import org.apache.calcite.sql.dialect.CalciteSqlDialect;
 import org.apache.calcite.sql.dialect.HiveSqlDialect;
 import org.apache.calcite.sql.dialect.JethroDataSqlDialect;
@@ -490,6 +491,30 @@ private static String toSql(RelNode root, SqlDialect dialect,
         .withPresto().ok(expected);
   }
 
+  /** When ceiling/flooring an integer, BigQuery returns a double while Calcite and other dialects
+   * return an integer. Therefore, casts to integer types should be preserved for BigQuery. */
+  @Test void testBigQueryCeilPreservesCast() {
+    final String query = "SELECT TIMESTAMP_SECONDS(CAST(CEIL(CAST(3 AS BIGINT)) AS BIGINT)) "
+        + "as created_thing\n FROM `foodmart`.`product`";
+    final SqlParser.Config parserConfig =
+        BigQuerySqlDialect.DEFAULT.configureParser(SqlParser.config());
+    final Sql sql = fixture()
+        .withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).parserConfig(parserConfig);
+    sql.withSql(query).ok("SELECT TIMESTAMP_SECONDS(CAST(CEIL(3) AS INT64)) AS "
+        + "created_thing\nFROM foodmart.product");
+  }
+
+  @Test void testBigQueryFloorPreservesCast() {
+    final String query = "SELECT TIMESTAMP_SECONDS(CAST(FLOOR(CAST(3 AS BIGINT)) AS BIGINT)) "
+        + "as created_thing\n FROM `foodmart`.`product`";
+    final SqlParser.Config parserConfig =
+        BigQuerySqlDialect.DEFAULT.configureParser(SqlParser.config());
+    final Sql sql = fixture()
+        .withBigQuery().withLibrary(SqlLibrary.BIG_QUERY).parserConfig(parserConfig);
+    sql.withSql(query).ok("SELECT TIMESTAMP_SECONDS(CAST(FLOOR(3) AS INT64)) AS "
+        + "created_thing\nFROM foodmart.product");
+  }
+
   @Test void testSelectLiteralAgg() {
     final Function<RelBuilder, RelNode> relFn = b -> b
         .scan("EMP")

diff --git a/site/_docs/reference.md b/site/_docs/reference.md
@@ -2673,6 +2673,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | SORT_ARRAY(array [, ascendingOrder])           | Sorts the *array* in ascending or descending order according to the natural ordering of the array elements. The default order is ascending if *ascendingOrder* is not specified. Null elements will be placed at the beginning of the returned array in ascending order or at the end of the returned array in descending order
 | * | ASINH(numeric)                                 | Returns the inverse hyperbolic sine of *numeric*
 | * | ATANH(numeric)                                 | Returns the inverse hyperbolic tangent of *numeric*
+| b | CEIL(value)                                    | Similar to standard `CEIL(value)` except if *value* is an integer type, the return type is a double
 | m s | CHAR(integer)                                | Returns the character whose ASCII code is *integer* % 256, or null if *integer* &lt; 0
 | b o p | CHR(integer)                               | Returns the character whose UTF-8 code is *integer*
 | o | CONCAT(string, string)                         | Concatenates two strings, returns null only when both string arguments are null, otherwise treats null as empty string
@@ -2715,6 +2716,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | o | EXTRACT(xml, xpath, [, namespaces ])           | Returns the XML fragment of the element or elements matched by the XPath expression. The optional namespace value that specifies a default mapping or namespace mapping for prefixes, which is used when evaluating the XPath expression
 | o | EXISTSNODE(xml, xpath, [, namespaces ])        | Determines whether traversal of a XML document using a specified xpath results in any nodes. Returns 0 if no nodes remain after applying the XPath traversal on the document fragment of the element or elements matched by the XPath expression. Returns 1 if any nodes remain. The optional namespace value that specifies a default mapping or namespace mapping for prefixes, which is used when evaluating the XPath expression.
 | m | EXTRACTVALUE(xml, xpathExpr))                  | Returns the text of the first text node which is a child of the element or elements matched by the XPath expression.
+| b | FLOOR(value)                                   | Similar to standard `FLOOR(value)` except if *value* is an integer type, the return type is a double
 | b | FORMAT_DATE(string, date)                      | Formats *date* according to the specified format *string*
 | b | FORMAT_DATETIME(string, timestamp)             | Formats *timestamp* according to the specified format *string*
 | b | FORMAT_TIME(string, time)                      | Formats *time* according to the specified format *string*

diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -8716,6 +8716,38 @@ private static void checkArrayConcatAggFuncFails(SqlOperatorFixture t) {
     f.checkNull("floor(cast(null as real))");
   }
 
+  @Test void testBigQueryCeilFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.checkType("ceil(cast(3 as tinyint))", "TINYINT NOT NULL");
+    final SqlOperatorFixture f = f0.setFor(SqlLibraryOperators.FLOOR_BIG_QUERY)
+        .withLibrary(SqlLibrary.BIG_QUERY).withConformance(SqlConformanceEnum.BIG_QUERY);
+    f.checkScalarExact("ceil(cast(3 as tinyint))", "DOUBLE", "3.0");
+    f.checkScalarExact("ceil(cast(3 as smallint))", "DOUBLE", "3.0");
+    f.checkScalarExact("ceil(cast(3 as integer))", "DOUBLE", "3.0");
+    f.checkScalarExact("ceil(cast(3 as bigint))", "DOUBLE", "3.0");
+    f.checkScalarExact("ceil(cast(3.5 as double))", "DOUBLE", "4.0");
+    f.checkScalarExact("ceil(cast(3.45 as decimal))",
+        "DECIMAL(19, 0)", "4");
+    f.checkScalarExact("ceil(cast(3.45 as float))", "FLOAT", "4.0");
+    f.checkNull("ceil(cast(null as tinyint))");
+  }
+
+  @Test void testBigQueryFloorFunc() {
+    final SqlOperatorFixture f0 = fixture();
+    f0.checkType("floor(cast(3 as tinyint))", "TINYINT NOT NULL");
+    final SqlOperatorFixture f = f0.setFor(SqlLibraryOperators.FLOOR_BIG_QUERY)
+        .withLibrary(SqlLibrary.BIG_QUERY).withConformance(SqlConformanceEnum.BIG_QUERY);
+    f.checkScalarExact("floor(cast(3 as tinyint))", "DOUBLE", "3.0");
+    f.checkScalarExact("floor(cast(3 as smallint))", "DOUBLE", "3.0");
+    f.checkScalarExact("floor(cast(3 as integer))", "DOUBLE", "3.0");
+    f.checkScalarExact("floor(cast(3 as bigint))", "DOUBLE", "3.0");
+    f.checkScalarExact("floor(cast(3.5 as double))", "DOUBLE", "3.0");
+    f.checkScalarExact("floor(cast(3.45 as decimal))",
+        "DECIMAL(19, 0)", "3");
+    f.checkScalarExact("floor(cast(3.45 as float))", "FLOAT", "3.0");
+    f.checkNull("floor(cast(null as tinyint))");
+  }
+
   @Test void testFloorFuncDateTime() {
     final SqlOperatorFixture f = fixture();
     f.enableTypeCoercion(false)