From e2982b2110b047ae3ede46eb2621d7bfb68a70b3 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Mon, 2 Dec 2019 16:05:05 +0200 Subject: [PATCH] SQL: handle NULL arithmetic operations with INTERVALs (#49633) (cherry picked from commit ce727615c08cf5ae422feb77f69ea24fb53cd9d1) --- .../sql/functions/date-time.asciidoc | 2 +- .../qa/src/main/resources/arithmetic.csv-spec | 9 +++++ .../main/resources/datetime-interval.csv-spec | 9 +++++ .../xpack/sql/expression/TypeResolutions.java | 7 ++-- .../sql/expression/function/scalar/Cast.java | 3 +- .../predicate/conditional/Case.java | 2 +- .../expression/predicate/nulls/IsNotNull.java | 5 +-- .../expression/predicate/nulls/IsNull.java | 5 +-- .../DateTimeArithmeticOperation.java | 2 +- .../predicate/operator/arithmetic/Mul.java | 6 +-- .../xpack/sql/querydsl/query/TermsQuery.java | 3 +- .../xpack/sql/type/DataType.java | 12 ++++++ .../xpack/sql/type/DataTypeConversion.java | 4 +- .../xpack/sql/type/DataTypes.java | 4 -- .../arithmetic/BinaryArithmeticTests.java | 39 +++++++++++++++++++ 15 files changed, 86 insertions(+), 26 deletions(-) diff --git a/docs/reference/sql/functions/date-time.asciidoc b/docs/reference/sql/functions/date-time.asciidoc index c3158244ec2..74a58da8fa3 100644 --- a/docs/reference/sql/functions/date-time.asciidoc +++ b/docs/reference/sql/functions/date-time.asciidoc @@ -55,7 +55,7 @@ s|Description ==== Operators -Basic arithmetic operators (`+`, `-`, etc) support date/time parameters as indicated below: +Basic arithmetic operators (`+`, `-`, `*`) support date/time parameters as indicated below: [source, sql] -------------------------------------------------- diff --git a/x-pack/plugin/sql/qa/src/main/resources/arithmetic.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/arithmetic.csv-spec index 9cbf544fd8d..31b2417932a 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/arithmetic.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/arithmetic.csv-spec @@ -18,3 +18,12 @@ SELECT 5 - 2 x FROM test_emp LIMIT 5; 3 ; + +nullArithmetics +schema::a:i|b:d|c:s|d:s|e:l|f:i|g:i|h:i|i:i|j:i|k:d +SELECT null + 2 AS a, null * 1.5 AS b, null + null AS c, null - null AS d, null - 1234567890123 AS e, 123 - null AS f, null / 5 AS g, 5 / null AS h, null % 5 AS i, 5 % null AS j, null + 5.5 - (null * (null * 3)) AS k; + + a | b | c | d | e | f | g | h | i | j | k +---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+--------------- +null |null |null |null |null |null |null |null |null |null |null +; diff --git a/x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec index 9bb89408923..785ea8f404d 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec @@ -191,6 +191,15 @@ SELECT 4 * -INTERVAL '2' HOURS AS result1, -5 * -INTERVAL '3' HOURS AS result2; -0 08:00:00.0 | +0 15:00:00.0 ; +intervalNullMath +schema::null_multiply:string|null_sub1:string|null_sub2:string|null_add:string +SELECT null * INTERVAL '1 23:45' DAY TO MINUTES AS null_multiply, INTERVAL '1' DAY - null AS null_sub1, null - INTERVAL '1' DAY AS null_sub2, INTERVAL 1 DAY + null AS null_add; + + null_multiply | null_sub1 | null_sub2 | null_add +-----------------+-------------+-------------+------------- +null |null |null |null +; + intervalAndFieldMultiply schema::languages:byte|result:string SELECT languages, CAST (languages * INTERVAL '1 10:30' DAY TO MINUTES AS string) AS result FROM test_emp ORDER BY emp_no LIMIT 5; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java index c465ab1b2de..30041ea1222 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypeResolutions.java @@ -5,8 +5,9 @@ */ package org.elasticsearch.xpack.sql.expression; +import org.elasticsearch.xpack.sql.expression.Expression.TypeResolution; +import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.type.DataType; -import org.elasticsearch.xpack.sql.type.DataTypes; import org.elasticsearch.xpack.sql.type.EsField; import java.util.Locale; @@ -14,8 +15,6 @@ import java.util.StringJoiner; import java.util.function.Predicate; import static org.elasticsearch.common.logging.LoggerMessageFormat.format; -import static org.elasticsearch.xpack.sql.expression.Expression.TypeResolution; -import static org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import static org.elasticsearch.xpack.sql.expression.Expressions.name; import static org.elasticsearch.xpack.sql.type.DataType.BOOLEAN; @@ -119,7 +118,7 @@ public final class TypeResolutions { String operationName, ParamOrdinal paramOrd, String... acceptedTypes) { - return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())? + return predicate.test(e.dataType()) || e.dataType().isNull() ? TypeResolution.TYPE_RESOLVED : new TypeResolution(format(null, "{}argument of [{}] must be [{}], found value [{}] type [{}]", paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : paramOrd.name().toLowerCase(Locale.ROOT) + " ", diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java index fd82d2bb4db..c3c0da05709 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java @@ -13,7 +13,6 @@ import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.DataTypeConversion; -import org.elasticsearch.xpack.sql.type.DataTypes; import java.util.Objects; @@ -64,7 +63,7 @@ public class Cast extends UnaryScalarFunction { @Override public Nullability nullable() { - if (DataTypes.isNull(from())) { + if (from().isNull()) { return Nullability.TRUE; } return field().nullable(); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Case.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Case.java index 1354bb27034..84f17283e06 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Case.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/Case.java @@ -78,7 +78,7 @@ public class Case extends ConditionalFunction { protected TypeResolution resolveType() { DataType expectedResultDataType = null; for (IfConditional ifConditional : conditions) { - if (DataTypes.isNull(ifConditional.result().dataType()) == false) { + if (ifConditional.result().dataType().isNull() == false) { expectedResultDataType = ifConditional.result().dataType(); break; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java index f43e12e0b40..53bf9bcc805 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNotNull.java @@ -12,10 +12,9 @@ import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; import org.elasticsearch.xpack.sql.expression.gen.script.Scripts; import org.elasticsearch.xpack.sql.expression.predicate.Negatable; import org.elasticsearch.xpack.sql.expression.predicate.nulls.CheckNullProcessor.CheckNullOperation; -import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.tree.NodeInfo; +import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; -import org.elasticsearch.xpack.sql.type.DataTypes; public class IsNotNull extends UnaryScalarFunction implements Negatable { @@ -35,7 +34,7 @@ public class IsNotNull extends UnaryScalarFunction implements Negatable { @@ -35,7 +34,7 @@ public class IsNull extends UnaryScalarFunction implements Negatable values) { super(source); this.term = term; - values.removeIf(e -> DataTypes.isNull(e.dataType())); + values.removeIf(e -> e.dataType().isNull()); if (values.isEmpty()) { this.values = Collections.emptySet(); } else { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java index 22f2a596e35..19fda28f76f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java @@ -247,6 +247,18 @@ public enum DataType { return isNumeric(); } + public boolean isNull() { + return this == NULL; + } + + public boolean isNullOrNumeric() { + return isNull() || isNumeric(); + } + + public boolean isNullOrInterval() { + return isNull() || isInterval(); + } + public boolean isString() { return this == KEYWORD || this == TEXT; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java index a9b836da135..8e3158c3e6a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java @@ -45,10 +45,10 @@ public abstract class DataTypeConversion { if (left == right) { return left; } - if (DataTypes.isNull(left)) { + if (left.isNull()) { return right; } - if (DataTypes.isNull(right)) { + if (right.isNull()) { return left; } if (left.isString() && right.isString()) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java index 5d4691e70d5..1faf19e1d09 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java @@ -31,10 +31,6 @@ public final class DataTypes { private DataTypes() {} - public static boolean isNull(DataType from) { - return from == NULL; - } - public static boolean isUnsupported(DataType from) { return from == UNSUPPORTED; } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/BinaryArithmeticTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/BinaryArithmeticTests.java index 1c4b0697f95..1b877cb75f3 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/BinaryArithmeticTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/BinaryArithmeticTests.java @@ -227,6 +227,45 @@ public class BinaryArithmeticTests extends ESTestCase { Period p = interval.interval(); assertEquals(Period.ofYears(2).negated(), p); } + + public void testMulNullInterval() { + Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH); + Mul result = new Mul(EMPTY, L(null), literal); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + + result = new Mul(EMPTY, literal, L(null)); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + } + + public void testAddNullInterval() { + Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH); + Add result = new Add(EMPTY, L(null), literal); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + + result = new Add(EMPTY, literal, L(null)); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + } + + public void testSubNullInterval() { + Literal literal = interval(Period.ofMonths(1), INTERVAL_MONTH); + Sub result = new Sub(EMPTY, L(null), literal); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + + result = new Sub(EMPTY, literal, L(null)); + assertTrue(result.foldable()); + assertNull(result.fold()); + assertEquals(INTERVAL_MONTH, result.dataType()); + } @SuppressWarnings("unchecked") private static T add(Object l, Object r) {