zoudan commented on code in PR #3234:
URL: https://github.com/apache/calcite/pull/3234#discussion_r1211015491
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6111,6 +6111,31 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("truncate(cast(null as double))");
}
+ @Test void testSafeMultiplyFunc() {
+ final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_MULTIPLY);
+ // Bounds of int64
+ final String max = "9223372036854775807";
Review Comment:
use `Long.MAX_VALUE` may be better
##########
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:
we do not have to use `@PolyNull` here？
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6111,6 +6111,31 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("truncate(cast(null as double))");
}
+ @Test void testSafeMultiplyFunc() {
+ final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_MULTIPLY);
+ // Bounds of int64
+ final String max = "9223372036854775807";
+ final String min = "-9223372036854775808";
+ 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);
+ // Most tests are to verify correct return type and handling of overflow
result
+ f.checkScalar("safe_multiply(2, 3)", "6", "INTEGER");
+ f.checkScalar("safe_multiply(2, -1)", "-2", "INTEGER");
+ f.checkScalar("safe_multiply(2.5, 3.5)", "8.75", "DECIMAL(4, 2)");
+ f.checkScalar("safe_multiply(cast(2 as float), 3)", "6.0", "FLOAT");
+
+ // Test that a result larger than INTEGER but less than overflow will
return BIGINT type
+ f.checkScalar("safe_multiply(1, " + max + ")", max, "BIGINT");
+ f.checkScalar("safe_multiply(1, " + min + ")", min, "BIGINT");
+
+ // Test that overflow/underflow/null operand correctly returns null
+ f.checkNull("safe_multiply(2, " + max + ")");
Review Comment:
shall we add more test cases for overflow of other types such as
safe_multiply(int b0, int b1), which should return null if the result is
greater than int max.
##########
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:
We just have to handle BigDecimal * BigDecimal？May be we should also handle
all the combination of input 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]
