julianhyde commented on code in PR #3234:
URL: https://github.com/apache/calcite/pull/3234#discussion_r1217257420
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1524,6 +1524,19 @@ 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 b0) {
+ return b0.compareTo(BigDecimal.valueOf(Long.MIN_VALUE)) < 0
+ || b0.compareTo(BigDecimal.valueOf(Long.MAX_VALUE)) > 0;
+ }
+
+ /** SQL {@code SAFE_MULTIPLY(<value0>, <value1>)} function. */
+ public static @Nullable Double safeMultiply(@PolyNull BigDecimal b0,
Review Comment:
Most of the java functions that implement SQL functions do not need to
handle null values. So I wouldn't expect to see `@PolyNull` unless there's a
good reason.
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1524,6 +1524,19 @@ 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 b0) {
+ return b0.compareTo(BigDecimal.valueOf(Long.MIN_VALUE)) < 0
+ || b0.compareTo(BigDecimal.valueOf(Long.MAX_VALUE)) > 0;
+ }
+
+ /** SQL {@code SAFE_MULTIPLY(<value0>, <value1>)} function. */
+ public static @Nullable Double safeMultiply(@PolyNull BigDecimal b0,
Review Comment:
I think we will need to implement `float * float` (32 bit FP), `double *
double` (64 bit FP), `long * long` (64 bit integer), etc.
Detection of overflow is tricky. I would lean on libraries such as
`java.math`. They may be able to tap into some of the CPU internals to detect
overflow.
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1524,6 +1524,139 @@ 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 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 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 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));
Review Comment:
I'm also surprised that you use `Float` (rather than `Double`). Each SQL
type should use the overflow checking for its corresponding Java data type:
SMALLINT = short, INTEGER = int, BIGINT = long, REAL = float, DOUBLE = double.
(`TINYINT` is tricky because it is signed whereas `byte` is unsigned.)
Note that BigQuery `FLOAT64` matches SQL `DOUBLE` and Java `double`.
I think you should have a version of `safeMultiply` for each java type, e.g.
`(short, short)`, `(int, int)`, `(long, long)`, `(float, float)`, `(double,
double)`. I don't think you need `BigDecimal`.
For `int` and `long` call `Math.multiplyExact` and catch the exception.
For `short`, call the `int` version and check whether the result is in range.
For `float` and `double` I don't know how you detect overflow. Maybe do
ordinary multiplication for now.
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1524,6 +1524,19 @@ 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 b0) {
+ return b0.compareTo(BigDecimal.valueOf(Long.MIN_VALUE)) < 0
+ || b0.compareTo(BigDecimal.valueOf(Long.MAX_VALUE)) > 0;
+ }
+
+ /** SQL {@code SAFE_MULTIPLY(<value0>, <value1>)} function. */
+ public static @Nullable Double safeMultiply(@PolyNull BigDecimal b0,
Review Comment:
`Math.multiplyExact' is the kind of thing I was thinking of:
https://stackoverflow.com/questions/1657834/how-can-i-check-if-multiplying-two-numbers-in-java-will-cause-an-overflow
--
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]
