From edbedfac6ad7cc4c91df47980a285d9b31ab22b1 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Thu, 3 Feb 2022 20:15:42 +0100 Subject: [PATCH 1/3] Add overflow check to interval multiplications This adds checks on the multiplication operation on intervals (with integer). --- .../SqlBinaryArithmeticOperation.java | 19 +++++++----- .../arithmetic/SqlBinaryArithmeticTests.java | 31 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java index 780ed998ff94a..252885df2ba05 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java @@ -13,6 +13,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Arithmetics; import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Arithmetics.NumericArithmetic; import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.BinaryArithmeticOperation; +import org.elasticsearch.xpack.ql.type.DataTypeConverter; import org.elasticsearch.xpack.sql.expression.literal.interval.Interval; import org.elasticsearch.xpack.sql.expression.literal.interval.IntervalArithmetics; import org.elasticsearch.xpack.sql.expression.literal.interval.IntervalDayTime; @@ -24,6 +25,8 @@ import java.time.temporal.Temporal; import java.util.function.BiFunction; +import static org.elasticsearch.xpack.ql.type.DataTypeConverter.safeToLong; + public enum SqlBinaryArithmeticOperation implements BinaryArithmeticOperation { ADD((Object l, Object r) -> { @@ -85,17 +88,17 @@ public enum SqlBinaryArithmeticOperation implements BinaryArithmeticOperation { if (l instanceof Number && r instanceof Number) { return Arithmetics.mul((Number) l, (Number) r); } - if (l instanceof Number && r instanceof IntervalYearMonth) { - return ((IntervalYearMonth) r).mul(((Number) l).intValue()); + if (l instanceof Number number && r instanceof IntervalYearMonth) { + return ((IntervalYearMonth) r).mul(safeToLong(number)); } - if (r instanceof Number && l instanceof IntervalYearMonth) { - return ((IntervalYearMonth) l).mul(((Number) r).intValue()); + if (r instanceof Number number && l instanceof IntervalYearMonth) { + return ((IntervalYearMonth) l).mul(safeToLong(number)); } - if (l instanceof Number && r instanceof IntervalDayTime) { - return ((IntervalDayTime) r).mul(((Number) l).longValue()); + if (l instanceof Number number && r instanceof IntervalDayTime) { + return ((IntervalDayTime) r).mul(safeToLong(number)); } - if (r instanceof Number && l instanceof IntervalDayTime) { - return ((IntervalDayTime) l).mul(((Number) r).longValue()); + if (r instanceof Number number && l instanceof IntervalDayTime) { + return ((IntervalDayTime) l).mul(safeToLong(number)); } throw new QlIllegalArgumentException( diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticTests.java index dd7ae3677e10c..09f814da8e5ef 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticTests.java @@ -24,6 +24,7 @@ import static org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Arithmetics.mod; import static org.elasticsearch.xpack.ql.tree.Source.EMPTY; +import static org.elasticsearch.xpack.ql.util.NumericUtils.UNSIGNED_LONG_MAX; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.INTERVAL_DAY; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.INTERVAL_DAY_TO_HOUR; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.INTERVAL_HOUR; @@ -243,6 +244,36 @@ public void testMulNullInterval() { assertEquals(INTERVAL_MONTH, result.dataType()); } + public void testMulIntegerIntervalYearMonthOverflow() { + Literal l = interval(Period.ofYears(1).plusMonths(11), INTERVAL_YEAR); + ArithmeticException expect = expectThrows(ArithmeticException.class, () -> mul(l, L(Integer.MAX_VALUE))); + assertEquals("integer overflow", expect.getMessage()); + } + + public void testMulLongIntervalYearMonthOverflow() { + Literal l = interval(Period.ofYears(1), INTERVAL_YEAR); + QlIllegalArgumentException expect = expectThrows(QlIllegalArgumentException.class, () -> mul(l, L(Long.MAX_VALUE))); + assertEquals("[9223372036854775807] out of [integer] range", expect.getMessage()); + } + + public void testMulUnsignedLongIntervalYearMonthOverflow() { + Literal l = interval(Period.ofYears(1), INTERVAL_YEAR); + QlIllegalArgumentException expect = expectThrows(QlIllegalArgumentException.class, () -> mul(l, L(UNSIGNED_LONG_MAX))); + assertEquals("[18446744073709551615] out of [long] range", expect.getMessage()); + } + + public void testMulLongIntervalDayTimeOverflow() { + Literal l = interval(Duration.ofDays(1), INTERVAL_DAY); + ArithmeticException expect = expectThrows(ArithmeticException.class, () -> mul(l, L(Long.MAX_VALUE))); + assertEquals("Exceeds capacity of Duration: 796899343984252629724800000000000", expect.getMessage()); + } + + public void testMulUnsignedLongIntervalDayTimeOverflow() { + Literal l = interval(Duration.ofDays(1), INTERVAL_DAY); + QlIllegalArgumentException expect = expectThrows(QlIllegalArgumentException.class, () -> mul(l, L(UNSIGNED_LONG_MAX))); + assertEquals("[18446744073709551615] out of [long] range", expect.getMessage()); + } + public void testAddNullInterval() { Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH); Add result = new Add(EMPTY, L(null), literal); From 7b03f79f5302ec392e74d15decf0b0f558c1d33d Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Thu, 3 Feb 2022 20:20:31 +0100 Subject: [PATCH 2/3] Update docs/changelog/83478.yaml --- docs/changelog/83478.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/83478.yaml diff --git a/docs/changelog/83478.yaml b/docs/changelog/83478.yaml new file mode 100644 index 0000000000000..84039e3f1fe82 --- /dev/null +++ b/docs/changelog/83478.yaml @@ -0,0 +1,6 @@ +pr: 83478 +summary: Add range checks to interval multiplication operation +area: SQL +type: bug +issues: + - 83336 From c0e677ef413008502330858ba2641119517c1ed7 Mon Sep 17 00:00:00 2001 From: Bogdan Pintea Date: Thu, 3 Feb 2022 20:33:03 +0100 Subject: [PATCH 3/3] Style fix Remove redundant import. --- .../operator/arithmetic/SqlBinaryArithmeticOperation.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java index 252885df2ba05..7c4aa8073147c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/SqlBinaryArithmeticOperation.java @@ -13,7 +13,6 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Arithmetics; import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Arithmetics.NumericArithmetic; import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.BinaryArithmeticOperation; -import org.elasticsearch.xpack.ql.type.DataTypeConverter; import org.elasticsearch.xpack.sql.expression.literal.interval.Interval; import org.elasticsearch.xpack.sql.expression.literal.interval.IntervalArithmetics; import org.elasticsearch.xpack.sql.expression.literal.interval.IntervalDayTime;