tanclary commented on code in PR #3234:
URL: https://github.com/apache/calcite/pull/3234#discussion_r1253716358
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6114,35 +6114,62 @@ private static void checkIf(SqlOperatorFixture f) {
@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);
+ "No match found for function signature "
+ + "SAFE_MULTIPLY\\(<NUMERIC>, <NUMERIC>\\)", false);
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
- // check multiplication with no over/underflow has correct result and
result type
- f.checkScalar("safe_multiply(cast(2 as int), cast(3 as int))",
- "6", "INTEGER");
- f.checkScalar("safe_multiply(cast(2 as int), cast(3.5 as double))",
- "7.0", "DOUBLE");
- f.checkScalar("safe_multiply(cast(2 as int), cast(3.456 as float))",
- "6.912", "FLOAT");
- f.checkScalar("safe_multiply(cast(2 as int), cast(3.456 as decimal(4,
3)))",
- "6.912", "DECIMAL(14, 3)");
- f.checkScalar("safe_multiply(cast(2 as double), cast(3.5 as double))",
- "7.0", "DOUBLE");
- // DOUBLE and FLOAT in Calcite maps to BigQuery "FLOAT64", so returning
DOUBLE is okay here
- f.checkScalar("safe_multiply(cast(2 as double), cast(3.456 as float))",
- "6.912", "DOUBLE");
- f.checkScalar("safe_multiply(cast(2 as double), cast(3.456 as decimal(4,
3)))",
- "6.912", "DOUBLE");
- f.checkScalar("safe_multiply(cast(2 as float), cast(3.456 as float))",
- "6.912", "FLOAT");
- f.checkScalar("safe_multiply(cast(2 as float), cast(3.456 as decimal(4,
3)))",
- "6.912", "DOUBLE");
- f.checkScalar("safe_multiply(cast(3.4 as decimal(4,3)), cast(3.456 as
decimal(4,3)))",
- "11.7504", "DECIMAL(8, 6)");
- // check overflow returns null
- f.checkNull("safe_multiply(cast(" + Integer.MAX_VALUE + " as int), 3)");
- f.checkNull("safe_multiply(cast(" + Double.MAX_VALUE + " as double), 3)");
- f.checkNull("safe_multiply(9223372036854775807, 3)");
+ // 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");
+ f.checkScalar("safe_multiply(cast(20 as bigint), cast(1.2345 as
decimal(5,4)))",
+ "24.6900", "DECIMAL(19, 4)");
+ f.checkScalar("safe_multiply(cast(1.2345 as decimal(5,4)), cast(20 as
bigint))",
+ "24.6900", "DECIMAL(19, 4)");
+ f.checkScalar("safe_multiply(cast(1.2345 as decimal(5,4)), "
+ + "cast(2.0 as decimal(2, 1)))", "2.46900",
+ "DECIMAL(7, 5)");
+ f.checkScalar("safe_multiply(cast(3 as float), cast(3 as bigint))",
Review Comment:
These should have been `Double` that's a mistake on my part, good catch.
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1524,136 +1524,89 @@ public static int multiply(int b0, int b1) {
throw notArithmetic("*", b0, b1);
}
- /** Helper method for safe arithmetic functions that checks if overflow has
occurred. */
- private static boolean isOverflow(BigDecimal ans, BigDecimal min, BigDecimal
max) {
- return ans.compareTo(min) > 0 || ans.compareTo(max) < 0;
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to integer values. */
- public static @Nullable Integer safeMultiply(int b0, int b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow = isOverflow(ans, BigDecimal.valueOf(Integer.MAX_VALUE),
BigDecimal.valueOf(Integer.MIN_VALUE));
- return overflow ? null : ans.intValue();
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to integer and double
values. */
- public static @Nullable Double safeMultiply(int b0, double b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Double.MAX_VALUE),
BigDecimal.valueOf(-Double.MAX_VALUE));
- return overflow ? null : ans.doubleValue();
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to integer and
BigDecimal values. */
- public static @Nullable BigDecimal safeMultiply(int b0, BigDecimal b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), b1);
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to integer and long
values. */
- public static @Nullable Long safeMultiply(int b0, long b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Long.MAX_VALUE),
BigDecimal.valueOf(Long.MIN_VALUE));
- return overflow ? null : ans.longValue();
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to double and integer
values. */
- public static @Nullable Double safeMultiply(double b0, int b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Double.MAX_VALUE),
BigDecimal.valueOf(-Double.MAX_VALUE));
- return overflow ? null : ans.doubleValue();
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to double values. */
- public static @Nullable Double safeMultiply(double b0, double b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Double.MAX_VALUE),
BigDecimal.valueOf(-Double.MAX_VALUE));
- return overflow ? null : ans.doubleValue();
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to double and BigDecimal
values. */
- public static @Nullable BigDecimal safeMultiply(double b0, BigDecimal b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), b1);
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
- }
-
- /** SQL <code>SAFE_MULTIPLY</code> function applied to double and long
values. */
- public static @Nullable Long safeMultiply(double b0, long b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Long.MAX_VALUE),
BigDecimal.valueOf(Long.MIN_VALUE));
- return overflow ? null : ans.longValue();
+ /** 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 BigDecimal and
integer values. */
- public static @Nullable BigDecimal safeMultiply(BigDecimal b0, int b1) {
- BigDecimal ans = multiply(b0, BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
+ /** 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 decimalOverflow(ans) ? ans : null;
}
- /** SQL <code>SAFE_MULTIPLY</code> function applied to BigDecimal and double
values. */
- public static @Nullable BigDecimal safeMultiply(BigDecimal b0, double b1) {
- BigDecimal ans = multiply(b0, BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
+ /** 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 decimalOverflow(ans) ? ans : null;
}
/** SQL <code>SAFE_MULTIPLY</code> function applied to BigDecimal values. */
public static @Nullable BigDecimal safeMultiply(BigDecimal b0, BigDecimal
b1) {
- BigDecimal ans = multiply(b0, b1);
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
+ BigDecimal ans = b0.multiply(b1);
+ return decimalOverflow(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 = multiply(b0, BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
+ /** 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;
+ boolean isInfinite = Double.isInfinite(b0);
+ return doubleOverflow(ans) || isInfinite ? ans : null;
}
- /** SQL <code>SAFE_MULTIPLY</code> function applied to long and integer
values. */
- public static @Nullable Long safeMultiply(long b0, int b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Long.MAX_VALUE),
BigDecimal.valueOf(Long.MIN_VALUE));
- return overflow ? null : ans.longValue();
+ /** 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;
+ boolean isInfinite = Double.isInfinite(b1);
+ return doubleOverflow(ans) || isInfinite ? ans : null;
}
- /** SQL <code>SAFE_MULTIPLY</code> function applied to long and double
values. */
- public static @Nullable Long safeMultiply(long b0, double b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Long.MAX_VALUE),
BigDecimal.valueOf(Long.MIN_VALUE));
- return overflow ? null : ans.longValue();
+ /** 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();
+ boolean isInfinite = Double.isInfinite(b0);
+ return doubleOverflow(ans) || isInfinite ? ans : null;
}
- /** SQL <code>SAFE_MULTIPLY</code> function applied to long values. */
- public static @Nullable Long safeMultiply(long b0, long b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), BigDecimal.valueOf(b1));
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Long.MAX_VALUE),
BigDecimal.valueOf(Long.MIN_VALUE));
- return overflow ? null : ans.longValue();
+ /** 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;
+ boolean isInfinite = Double.isInfinite(b1);
+ return doubleOverflow(ans) || isInfinite ? ans : null;
}
- /** SQL <code>SAFE_MULTIPLY</code> function applied to long and BigDecimal
values. */
- public static @Nullable BigDecimal safeMultiply(long b0, BigDecimal b1) {
- BigDecimal ans = multiply(BigDecimal.valueOf(b0), b1);
- boolean overflow =
- isOverflow(ans, BigDecimal.valueOf(Float.MAX_VALUE),
BigDecimal.valueOf(-Float.MAX_VALUE));
- return overflow ? null : ans;
+ /** SQL <code>SAFE_MULTIPLY</code> function applied to double values. */
+ public static @Nullable Double safeMultiply(double b0, double b1) {
+ double ans = b0 * b1;
+ boolean isInfinite = Double.isInfinite(b0) || Double.isInfinite(b1);
+ return doubleOverflow(ans) || isInfinite ? 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 precision
+ * is greater than 38. */
Review Comment:
The behavior is correct, the naming is just a bit confusing. I changed it to
`safeDouble` and `safeDecimal` which return `TRUE` if there is not overflow and
`FALSE` if there is. Let me know if you agree with this.
--
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]
