julianhyde commented on code in PR #3234:
URL: https://github.com/apache/calcite/pull/3234#discussion_r1280948889


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6843,6 +6843,79 @@ private static void checkIf(SqlOperatorFixture f) {
     f.checkNull("truncate(cast(null as double))");
   }
 
+  @Test void testSafeMultiplyFunc() {
+    final SqlOperatorFixture f0 = 
fixture().setFor(SqlLibraryOperators.SAFE_MULTIPLY);
+    f0.checkFails("^safe_multiply(2, 3)^",
+        "No match found for function signature "
+        + "SAFE_MULTIPLY\\(<NUMERIC>, <NUMERIC>\\)", false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+    // Basic test for each of the 9 2-permutations of BIGINT, DECIMAL, and 
FLOAT
+    f.checkScalar("safe_multiply(cast(20 as bigint), cast(20 as bigint))",
+                  "400", "BIGINT");

Review Comment:
   method arguments should be indented 4, not aligned with open paren. please 
fix throughout this method



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1592,6 +1592,87 @@ public static int multiply(int b0, int b1) {
     throw notArithmetic("*", b0, b1);
   }
 
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to long values. */
+  public static @Nullable Long safeMultiply(long b0, long b1) {
+    try {
+      return Math.multiplyExact(b0, b1);
+    } catch (ArithmeticException e) {
+      return null;
+    }
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to long and BigDecimal 
values. */
+  public static @Nullable BigDecimal safeMultiply(long b0, BigDecimal b1) {
+    BigDecimal ans = BigDecimal.valueOf(b0).multiply(b1);
+    return safeDecimal(ans) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to BigDecimal and long 
values. */
+  public static @Nullable BigDecimal safeMultiply(BigDecimal b0, long b1) {
+    BigDecimal ans = b0.multiply(BigDecimal.valueOf(b1));
+    return safeDecimal(ans) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to BigDecimal values. */
+  public static @Nullable BigDecimal safeMultiply(BigDecimal b0, BigDecimal 
b1) {
+    BigDecimal ans = b0.multiply(b1);
+    return safeDecimal(ans) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to double and long 
values. */
+  public static @Nullable Double safeMultiply(double b0, long b1) {
+    double ans = b0 * b1;
+    return safeDouble(ans) || !Double.isFinite(b0) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to long and double 
values. */
+  public static @Nullable Double safeMultiply(long b0, double b1) {
+    double ans = b0 * b1;
+    return safeDouble(ans) || !Double.isFinite(b1) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to double and BigDecimal 
values. */
+  public static @Nullable Double safeMultiply(double b0, BigDecimal b1) {
+    double ans = b0 * b1.doubleValue();
+    return safeDouble(ans) || !Double.isFinite(b0) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to BigDecimal and double 
values. */
+  public static @Nullable Double safeMultiply(BigDecimal b0, double b1) {
+    double ans = b0.doubleValue() * b1;
+    return safeDouble(ans) || !Double.isFinite(b1) ? ans : null;
+  }
+
+  /** SQL <code>SAFE_MULTIPLY</code> function applied to double values. */
+  public static @Nullable Double safeMultiply(double b0, double b1) {
+    double ans = b0 * b1;
+    boolean isFinite = Double.isFinite(b0) && Double.isFinite(b1);
+    return safeDouble(ans) || !isFinite ? ans : null;
+  }
+
+  /** Helper for the safe arithmetic functions to determine
+   * overflow for a BigDecimal value. According to BigQuery, BigDecimal
+   * overflow occurs if the precision is greater than 76 or the scale
+   * is greater than 38. */
+  private static boolean safeDecimal(BigDecimal b) {
+    return b.scale() <= 38 && b.precision() <= 76;
+  }
+
+  /** Helper for the safe arithmetic functions to determine

Review Comment:
   Comment is still misleading. The method doesn't 'determine overflow'. It 
determines that the result has *not* overflowed.
   
   Keep it simple. 'Returns whether a double value is safe (that is, has not 
overflowed).'



##########
site/_docs/reference.md:
##########
@@ -2779,6 +2779,7 @@ BigQuery's type system uses confusingly different names 
for types and functions:
 | b o | RPAD(string, length[, pattern ])             | Returns a string or 
bytes value that consists of *string* appended to *length* with *pattern*
 | b o | RTRIM(string)                                | Returns *string* with 
all blanks removed from the end
 | b | SAFE_CAST(value AS type)                       | Converts *value* to 
*type*, returning NULL if conversion fails
+| b | SAFE_MULTIPLY(numeric1, numeric2)              | Returns *numeric1* * 
*numeric2*, null is returned if overflow occurs

Review Comment:
   passive voice should be avoided
   
   don't use comma where grammar requires a semicolon
   
   I would write "Returns *numeric1* * *numeric2*, or null on overflow"



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -2369,6 +2372,30 @@ private static class LastDayImplementor extends 
MethodNameImplementor {
     }
   }
 
+  /** Implementor for the {@code SAFE_MULTIPLY} function. */
+  private static class SafeArithmeticImplementor extends MethodNameImplementor 
{
+    SafeArithmeticImplementor() {
+      super("safeMultiply", NullPolicy.STRICT, false);
+    }
+
+    @Override Expression implementSafe(final RexToLixTranslator translator,
+        final RexCall call, final List<Expression> argValueList) {
+      Expression arg0 = convertType(argValueList.get(0), call.operands.get(0));
+      Expression arg1 = convertType(argValueList.get(1), call.operands.get(1));
+      return Expressions.call(SqlFunctions.class, "safeMultiply", arg0, arg1);
+    }
+
+    // Because BigQuery treats all int types as aliases for BIGINT (Java's 
LONG)

Review Comment:
   s/LONG/long/
   
   (Uppercase type names connote SQL types.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to