SQL: handle NULL arithmetic operations with INTERVALs (elastic#49633)

astefan · SivagurunathanV · commit 5c216357f78c · 2020-01-20T19:31:34.000-08:00
diff --git 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
@@ -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
@@ -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
@@ -5,17 +5,16 @@
  */
 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;
 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 static TypeResolution isType(Expression e,
                                         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
@@ -13,7 +13,6 @@
 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 Object fold() {
 
     @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
@@ -78,7 +78,7 @@ protected NodeInfo<? extends Expression> info() {
     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
@@ -12,10 +12,9 @@
 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<UnaryScalarFunction> {
 
@@ -35,7 +34,7 @@ protected IsNotNull replaceChild(Expression newChild) {
 
     @Override
     public Object fold() {
-        return field().fold() != null && !DataTypes.isNull(field().dataType());
+        return field().fold() != null && !field().dataType().isNull();
     }
 
     @Override
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNull.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/nulls/IsNull.java
@@ -12,10 +12,9 @@
 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 IsNull extends UnaryScalarFunction implements Negatable<UnaryScalarFunction> {
 
@@ -35,7 +34,7 @@ protected IsNull replaceChild(Expression newChild) {
 
     @Override
     public Object fold() {
-        return field().fold() == null || DataTypes.isNull(field().dataType());
+        return field().fold() == null || field().dataType().isNull();
     }
 
     @Override
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java
@@ -56,7 +56,7 @@ protected TypeResolution resolveWithIntervals() {
         DataType l = left().dataType();
         DataType r = right().dataType();
 
-        if (!(r.isDateOrTimeBased() || r.isInterval())|| !(l.isDateOrTimeBased() || l.isInterval())) {
+        if (!(r.isDateOrTimeBased() || r.isInterval() || r.isNull())|| !(l.isDateOrTimeBased() || l.isInterval() || l.isNull())) {
             return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
         }
         return TypeResolution.TYPE_RESOLVED;
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Mul.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Mul.java
@@ -34,14 +34,14 @@ protected TypeResolution resolveType() {
         DataType r = right().dataType();
 
         // 1. both are numbers
-        if (l.isNumeric() && r.isNumeric()) {
+        if (l.isNullOrNumeric() && r.isNullOrNumeric()) {
             return TypeResolution.TYPE_RESOLVED;
         }
 
-        if (l.isInterval() && r.isInteger()) {
+        if (l.isNullOrInterval() && (r.isInteger() || r.isNull())) {
             dataType = l;
             return TypeResolution.TYPE_RESOLVED;
-        } else if (r.isInterval() && l.isInteger()) {
+        } else if (r.isNullOrInterval() && (l.isInteger() || l.isNull())) {
             dataType = r;
             return TypeResolution.TYPE_RESOLVED;
         }
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermsQuery.java
@@ -9,7 +9,6 @@
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Foldables;
 import org.elasticsearch.xpack.sql.tree.Source;
-import org.elasticsearch.xpack.sql.type.DataTypes;
 
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -27,7 +26,7 @@ public class TermsQuery extends LeafQuery {
     public TermsQuery(Source source, String term, List<Expression> 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
@@ -247,6 +247,18 @@ public boolean isSigned() {
         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
@@ -45,10 +45,10 @@ public static DataType commonType(DataType left, DataType right) {
         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
@@ -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
@@ -227,6 +227,45 @@ public void testMulNumberInterval() {
         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> T add(Object l, Object r) {

Original file line number	Diff line number	Diff line change
`@@ -18,3 +18,12 @@ SELECT 5 - 2 x FROM test_emp LIMIT 5;`
`18`	`18`	`3`
`19`	`19`	`;`
`20`	`20`
	`21`	`+`
	`22`	`+nullArithmetics`
	`23`	`+schema::a:i\|b:d\|c:s\|d:s\|e:l\|f:i\|g:i\|h:i\|i:i\|j:i\|k:d`
	`24`	`+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;`
	`25`	`+`
	`26`	`+ a \| b \| c \| d \| e \| f \| g \| h \| i \| j \| k`
	`27`	`+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------`
	`28`	`+null \|null \|null \|null \|null \|null \|null \|null \|null \|null \|null`
	`29`	`+;`
Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ protected NodeInfo<? extends Expression> info() {`
`78`	`78`	`protected TypeResolution resolveType() {`
`79`	`79`	`DataType expectedResultDataType = null;`
`80`	`80`	`for (IfConditional ifConditional : conditions) {`
`81`		`- if (DataTypes.isNull(ifConditional.result().dataType()) == false) {`
	`81`	`+ if (ifConditional.result().dataType().isNull() == false) {`
`82`	`82`	`expectedResultDataType = ifConditional.result().dataType();`
`83`	`83`	`break;`
`84`	`84`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ protected TypeResolution resolveWithIntervals() {`
`56`	`56`	`DataType l = left().dataType();`
`57`	`57`	`DataType r = right().dataType();`
`58`	`58`
`59`		`- if (!(r.isDateOrTimeBased() \|\| r.isInterval())\|\| !(l.isDateOrTimeBased() \|\| l.isInterval())) {`
	`59`	`+ if (!(r.isDateOrTimeBased() \|\| r.isInterval() \|\| r.isNull())\|\| !(l.isDateOrTimeBased() \|\| l.isInterval() \|\| l.isNull())) {`
`60`	`60`	`return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));`
`61`	`61`	`}`
`62`	`62`	`return TypeResolution.TYPE_RESOLVED;`
Original file line number	Diff line number	Diff line change
`@@ -45,10 +45,10 @@ public static DataType commonType(DataType left, DataType right) {`
`45`	`45`	`if (left == right) {`
`46`	`46`	`return left;`
`47`	`47`	`}`
`48`		`- if (DataTypes.isNull(left)) {`
	`48`	`+ if (left.isNull()) {`
`49`	`49`	`return right;`
`50`	`50`	`}`
`51`		`- if (DataTypes.isNull(right)) {`
	`51`	`+ if (right.isNull()) {`
`52`	`52`	`return left;`
`53`	`53`	`}`
`54`	`54`	`if (left.isString() && right.isString()) {`
Original file line number	Diff line number	Diff line change
`@@ -31,10 +31,6 @@ public final class DataTypes {`
`31`	`31`
`32`	`32`	`private DataTypes() {}`
`33`	`33`
`34`		`- public static boolean isNull(DataType from) {`
`35`		`- return from == NULL;`
`36`		`- }`
`37`		`-`
`38`	`34`	`public static boolean isUnsupported(DataType from) {`
`39`	`35`	`return from == UNSUPPORTED;`
`40`	`36`	`}`