From cbc634590755f041f26291de0e6f634e169a1635 Mon Sep 17 00:00:00 2001 From: librian415 Date: Sun, 12 Jan 2025 23:57:28 -0800 Subject: [PATCH 1/5] [Presto-Main] Add long overflow checks for IntervalDayTime --- .../presto/type/IntervalDayTimeOperators.java | 67 ++++++++++++++++--- .../presto/type/TestIntervalDayTime.java | 9 ++- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java b/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java index 15c92d2be7867..9eac48409a515 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java +++ b/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java @@ -16,6 +16,7 @@ import com.facebook.presto.common.block.Block; import com.facebook.presto.common.type.AbstractLongType; import com.facebook.presto.common.type.StandardTypes; +import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.function.BlockIndex; import com.facebook.presto.spi.function.BlockPosition; import com.facebook.presto.spi.function.IsNull; @@ -42,8 +43,11 @@ import static com.facebook.presto.common.function.OperatorType.NEGATION; import static com.facebook.presto.common.function.OperatorType.NOT_EQUAL; import static com.facebook.presto.common.function.OperatorType.SUBTRACT; +import static com.facebook.presto.spi.StandardErrorCode.DIVISION_BY_ZERO; +import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; import static com.facebook.presto.type.IntervalDayTimeType.INTERVAL_DAY_TIME; import static io.airlift.slice.Slices.utf8Slice; +import static java.lang.String.format; public final class IntervalDayTimeOperators { @@ -55,56 +59,103 @@ private IntervalDayTimeOperators() @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long add(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long left, @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long right) { - return left + right; + try { + return Math.addExact(left, right); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second addition overflow: %s ms + %s ms", left, right), e); + } } @ScalarOperator(SUBTRACT) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long subtract(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long left, @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long right) { - return left - right; + try { + return Math.subtractExact(left, right); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second subtraction overflow: %s ms - %s ms", left, right), e); + } } @ScalarOperator(MULTIPLY) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long multiplyByBigint(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long left, @SqlType(StandardTypes.BIGINT) long right) { - return left * right; + try { + return Math.multiplyExact(left, right); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second multiply overflow: %s ms * %s", left, right), e); + } } @ScalarOperator(MULTIPLY) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long multiplyByDouble(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long left, @SqlType(StandardTypes.DOUBLE) double right) { - return (long) (left * right); + try { + return Math.addExact( + (long) (left * (right - (long) right)), + Math.multiplyExact(left, (long) right)); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second multiply overflow: %s ms * %s", left, right), e); + } } @ScalarOperator(MULTIPLY) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long bigintMultiply(@SqlType(StandardTypes.BIGINT) long left, @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long right) { - return left * right; + try { + return Math.multiplyExact(left, right); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second multiply overflow: %s * %s ms", left, right), e); + } } @ScalarOperator(MULTIPLY) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long doubleMultiply(@SqlType(StandardTypes.DOUBLE) double left, @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long right) { - return (long) (left * right); + try { + return Math.addExact( + Math.multiplyExact((long) left, right), + (long) ((left - (long) left) * right)); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second multiply overflow: %s * %s ms", left, right), e); + } } @ScalarOperator(DIVIDE) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long divideByDouble(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long left, @SqlType(StandardTypes.DOUBLE) double right) { - return (long) (left / right); + if (right == 0) { + throw new PrestoException(DIVISION_BY_ZERO, format("interval_day_to_second division by zero: %s ms / %s", left, right)); + } + + try { + return multiplyByDouble(left, 1.0 / right); + } catch (PrestoException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second division overflow: %s ms / %s", left, right)); + } } @ScalarOperator(NEGATION) @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) public static long negate(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) long value) { - return -value; + try { + return Math.negateExact(value); + } + catch (ArithmeticException e) { + throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, "interval_day_to_second negation overflow: " + value, e); + } } @ScalarOperator(EQUAL) diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java index 36dddb114c1aa..39eefa8370d01 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java @@ -21,6 +21,7 @@ import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static com.facebook.presto.type.IntervalDayTimeType.INTERVAL_DAY_TIME; +import static java.lang.String.format; import static java.util.concurrent.TimeUnit.DAYS; import static org.testng.Assert.assertEquals; @@ -109,6 +110,7 @@ public void testAdd() assertFunction("INTERVAL '3' SECOND + INTERVAL '3' SECOND", INTERVAL_DAY_TIME, new SqlIntervalDayTime(6 * 1000)); assertFunction("INTERVAL '6' DAY + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime(12 * 24 * 60 * 60 * 1000)); assertFunction("INTERVAL '3' SECOND + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((6 * 24 * 60 * 60 * 1000) + (3 * 1000))); + assertFunction("INTERVAL '3' DAY + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((6 * 24 * 60 * 60 * 1000) + (3 * 1000))); } @Test @@ -116,7 +118,6 @@ public void testSubtract() { assertFunction("INTERVAL '6' SECOND - INTERVAL '3' SECOND", INTERVAL_DAY_TIME, new SqlIntervalDayTime(3 * 1000)); assertFunction("INTERVAL '9' DAY - INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime(3 * 24 * 60 * 60 * 1000)); - assertFunction("INTERVAL '3' SECOND - INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((3 * 1000) - (6 * 24 * 60 * 60 * 1000))); } @Test @@ -131,6 +132,9 @@ public void testMultiply() assertFunction("2 * INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime(12 * 24 * 60 * 60 * 1000)); assertFunction("INTERVAL '1' DAY * 2.5", INTERVAL_DAY_TIME, new SqlIntervalDayTime((long) (2.5 * 24 * 60 * 60 * 1000))); assertFunction("2.5 * INTERVAL '1' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((long) (2.5 * 24 * 60 * 60 * 1000))); + assertNumericOverflow( + format("%s * INTERVAL '%s' DAY", Integer.MAX_VALUE, 64), + format("interval_day_to_second multiply overflow: %s * %s ms", Integer.MAX_VALUE, (64 * 24 * 60 * 60 * 1000L))); } @Test @@ -141,6 +145,9 @@ public void testDivide() assertFunction("INTERVAL '3' DAY / 2", INTERVAL_DAY_TIME, new SqlIntervalDayTime((long) (1.5 * 24 * 60 * 60 * 1000))); assertFunction("INTERVAL '4' DAY / 2.5", INTERVAL_DAY_TIME, new SqlIntervalDayTime((long) (1.6 * 24 * 60 * 60 * 1000))); + assertNumericOverflow( + format("INTERVAL '%s' DAY / %s", 64, 1 / (double) Integer.MAX_VALUE), + format("interval_day_to_second division overflow: %s ms / %s", (64 * 24 * 60 * 60 * 1000L), 1 / (double) Integer.MAX_VALUE)); } @Test From ece59dbf3ef5f6d35fb0638ea50c7bc7f6add72f Mon Sep 17 00:00:00 2001 From: librian415 Date: Mon, 13 Jan 2025 10:07:26 -0800 Subject: [PATCH 2/5] lint --- .../com/facebook/presto/type/IntervalDayTimeOperators.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java b/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java index 9eac48409a515..c49edc9d2a99c 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java +++ b/presto-main/src/main/java/com/facebook/presto/type/IntervalDayTimeOperators.java @@ -141,7 +141,8 @@ public static long divideByDouble(@SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND) try { return multiplyByDouble(left, 1.0 / right); - } catch (PrestoException e) { + } + catch (PrestoException e) { throw new PrestoException(NUMERIC_VALUE_OUT_OF_RANGE, format("interval_day_to_second division overflow: %s ms / %s", left, right)); } } From ccf34f984c86ecc4088010fabf4e9e46761a0bf2 Mon Sep 17 00:00:00 2001 From: librian415 Date: Mon, 13 Jan 2025 10:57:14 -0800 Subject: [PATCH 3/5] remove leftover --- .../test/java/com/facebook/presto/type/TestIntervalDayTime.java | 1 - 1 file changed, 1 deletion(-) diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java index 39eefa8370d01..ce4a85b50f6c6 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java @@ -110,7 +110,6 @@ public void testAdd() assertFunction("INTERVAL '3' SECOND + INTERVAL '3' SECOND", INTERVAL_DAY_TIME, new SqlIntervalDayTime(6 * 1000)); assertFunction("INTERVAL '6' DAY + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime(12 * 24 * 60 * 60 * 1000)); assertFunction("INTERVAL '3' SECOND + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((6 * 24 * 60 * 60 * 1000) + (3 * 1000))); - assertFunction("INTERVAL '3' DAY + INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((6 * 24 * 60 * 60 * 1000) + (3 * 1000))); } @Test From bc02b0b79a78aa0b68c2dce7bdbbb0d323ed42fa Mon Sep 17 00:00:00 2001 From: librian415 Date: Wed, 15 Jan 2025 11:26:46 -0800 Subject: [PATCH 4/5] retrigger checks From 8559ab465ef6cc9e4a6591da9e9214e35652d3f2 Mon Sep 17 00:00:00 2001 From: librian415 Date: Thu, 16 Jan 2025 11:06:58 -0800 Subject: [PATCH 5/5] re-add deleted subtraction test --- .../test/java/com/facebook/presto/type/TestIntervalDayTime.java | 1 + 1 file changed, 1 insertion(+) diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java index ce4a85b50f6c6..4e09408e80546 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestIntervalDayTime.java @@ -117,6 +117,7 @@ public void testSubtract() { assertFunction("INTERVAL '6' SECOND - INTERVAL '3' SECOND", INTERVAL_DAY_TIME, new SqlIntervalDayTime(3 * 1000)); assertFunction("INTERVAL '9' DAY - INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime(3 * 24 * 60 * 60 * 1000)); + assertFunction("INTERVAL '3' SECOND - INTERVAL '6' DAY", INTERVAL_DAY_TIME, new SqlIntervalDayTime((3 * 1000) - (6 * 24 * 60 * 60 * 1000))); } @Test