Browse Source

SQL: Fix arg verification for DateAddProcessor (#48041)

Previously, the safety check for the 2nd argument of the DateAddProcessor was
restricting it to Integer which was wrong since we allow all non-rational
numbers, so it's changed to a Number check as it's done in other cases.

Enhanced some tests regarding the check for an integer (non-rational
argument).
Marios Trivyzas 6 years ago
parent
commit
0516b6eaf5

+ 4 - 0
docs/reference/sql/functions/date-time.asciidoc

@@ -274,6 +274,10 @@ if a negative value is used it results to a subtraction from the date/datetime
 Add the given number of date/time units to a date/datetime. If the number of units is negative then it's subtracted from
 the date/datetime. If any of the three arguments is `null` a `null` is returned.
 
+[WARNING]
+If the second argument is a long there is possibility of truncation since an integer value will be extracted and
+used from that long.
+
 [cols="^,^"]
 |===
 2+h|Datetime units to add/subtract

+ 19 - 0
x-pack/plugin/sql/qa/src/main/resources/datetime.csv-spec

@@ -170,6 +170,25 @@ FROM test_emp WHERE emp_no >= 10032 AND emp_no <= 10042 ORDER BY 1;
 10042     | null                     |  null                    |  null                    |  null                    |  null                    |  null                    |  null                    |  null
 ;
 
+selectAddWithLong
+schema::emp_no:i | birth_date:ts | date_add:date
+SELECT emp_no, birth_date, TIMESTAMP_ADD('months', CAST(CAST({fn TRUNCATE(11.55,0)} AS INTEGER) AS BIGINT), birth_date)::date AS date_add
+FROM test_emp ORDER BY emp_no LIMIT 10;
+
+ emp_no | birth_date               | date_add
+--------+--------------------------+-----------
+10001   | 1953-09-02 00:00:00.000Z | 1954-08-02
+10002   | 1964-06-02 00:00:00.000Z | 1965-05-02
+10003   | 1959-12-03 00:00:00.000Z | 1960-11-03
+10004   | 1954-05-01 00:00:00.000Z | 1955-04-01
+10005   | 1955-01-21 00:00:00.000Z | 1955-12-21
+10006   | 1953-04-20 00:00:00.000Z | 1954-03-20
+10007   | 1957-05-23 00:00:00.000Z | 1958-04-23
+10008   | 1958-02-19 00:00:00.000Z | 1959-01-19
+10009   | 1952-04-19 00:00:00.000Z | 1953-03-19
+10010   | 1963-06-01 00:00:00.000Z | 1964-05-01
+;
+
 selectAddWithComplexExpressions1
 SELECT gender, birth_date, TIMESTAMPADD('months', CASE WHEN gender = 'M' THEN 10 WHEN gender = 'F' THEN -10 ELSE 100 END,
 birth_date + INTERVAL 10 month) AS dt FROM test_emp WHERE dt > '1954-07-01'::date ORDER BY emp_no LIMIT 10;

+ 3 - 3
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateAddProcessor.java

@@ -59,14 +59,14 @@ public class DateAddProcessor extends ThreeArgsDateTimeProcessor {
             }
         }
 
-        if (numberOfUnits instanceof Integer == false) {
-            throw new SqlIllegalArgumentException("An integer is required; received [{}]", numberOfUnits);
+        if (numberOfUnits instanceof Number == false) {
+            throw new SqlIllegalArgumentException("A number is required; received [{}]", numberOfUnits);
         }
 
         if (timestamp instanceof ZonedDateTime == false) {
             throw new SqlIllegalArgumentException("A date/datetime is required; received [{}]", timestamp);
         }
 
-        return datePartField.add(((ZonedDateTime) timestamp).withZoneSameInstant(zoneId), (Integer) numberOfUnits);
+        return datePartField.add(((ZonedDateTime) timestamp).withZoneSameInstant(zoneId), ((Number) numberOfUnits).intValue());
     }
 }

+ 31 - 13
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

@@ -5,12 +5,17 @@
  */
 package org.elasticsearch.xpack.sql.analysis.analyzer;
 
+import org.elasticsearch.common.time.IsoLocale;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.TestUtils;
 import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
 import org.elasticsearch.xpack.sql.analysis.index.IndexResolverTests;
 import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
+import org.elasticsearch.xpack.sql.expression.function.scalar.math.Round;
+import org.elasticsearch.xpack.sql.expression.function.scalar.math.Truncate;
+import org.elasticsearch.xpack.sql.expression.function.scalar.string.Char;
+import org.elasticsearch.xpack.sql.expression.function.scalar.string.Space;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest;
 import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull;
@@ -23,6 +28,7 @@ import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.type.EsField;
 import org.elasticsearch.xpack.sql.type.TypesTests;
 
+import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
@@ -259,8 +265,8 @@ public class VerifierErrorMessagesTests extends ESTestCase {
     public void testDateAddInvalidArgs() {
         assertEquals("1:8: first argument of [DATE_ADD(int, int, date)] must be [string], found value [int] type [integer]",
             error("SELECT DATE_ADD(int, int, date) FROM test"));
-        assertEquals("1:8: second argument of [DATE_ADD(keyword, keyword, date)] must be [integer], found value [keyword] " +
-            "type [keyword]", error("SELECT DATE_ADD(keyword, keyword, date) FROM test"));
+        assertEquals("1:8: second argument of [DATE_ADD(keyword, 1.2, date)] must be [integer], found value [1.2] " +
+            "type [double]", error("SELECT DATE_ADD(keyword, 1.2, date) FROM test"));
         assertEquals("1:8: third argument of [DATE_ADD(keyword, int, keyword)] must be [date or datetime], found value [keyword] " +
             "type [keyword]", error("SELECT DATE_ADD(keyword, int, keyword) FROM test"));
         assertEquals("1:8: first argument of [DATE_ADD('invalid', int, date)] must be one of [YEAR, QUARTER, MONTH, DAYOFYEAR, " +
@@ -277,9 +283,9 @@ public class VerifierErrorMessagesTests extends ESTestCase {
 
     public void testDateAddValidArgs() {
         accept("SELECT DATE_ADD('weekday', 0, date) FROM test");
-        accept("SELECT DATE_ADD('dw', 20, date) FROM test");
-        accept("SELECT DATE_ADD('years', -10, date) FROM test");
-        accept("SELECT DATE_ADD('dayofyear', 123, date) FROM test");
+        accept("SELECT DATEADD('dw', 20, date) FROM test");
+        accept("SELECT TIMESTAMP_ADD('years', -10, date) FROM test");
+        accept("SELECT TIMESTAMPADD('dayofyear', 123, date) FROM test");
         accept("SELECT DATE_ADD('dy', 30, date) FROM test");
         accept("SELECT DATE_ADD('ms', 1, date::date) FROM test");
     }
@@ -554,13 +560,18 @@ public class VerifierErrorMessagesTests extends ESTestCase {
     }
 
     public void testInvalidTypeForStringFunction_WithOneArgNumeric() {
-        assertEquals("1:8: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]",
-            error("SELECT CHAR('foo')"));
+        String functionName = randomFrom(Arrays.asList(Char.class, Space.class)).getSimpleName().toUpperCase(IsoLocale.ROOT);
+        assertEquals("1:8: argument of [" + functionName + "('foo')] must be [integer], found value ['foo'] type [keyword]",
+            error("SELECT " + functionName + "('foo')"));
+        assertEquals("1:8: argument of [" + functionName + "(1.2)] must be [integer], found value [1.2] type [double]",
+            error("SELECT " + functionName + "(1.2)"));
     }
 
     public void testInvalidTypeForNestedStringFunctions_WithOneArg() {
-        assertEquals("1:14: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]",
-            error("SELECT ASCII(CHAR('foo'))"));
+        assertEquals("1:15: argument of [SPACE('foo')] must be [integer], found value ['foo'] type [keyword]",
+            error("SELECT LENGTH(SPACE('foo'))"));
+        assertEquals("1:15: argument of [SPACE(1.2)] must be [integer], found value [1.2] type [double]",
+            error("SELECT LENGTH(SPACE(1.2))"));
     }
 
     public void testInvalidTypeForNumericFunction_WithOneArg() {
@@ -581,10 +592,13 @@ public class VerifierErrorMessagesTests extends ESTestCase {
     }
 
     public void testInvalidTypeForNumericFunction_WithTwoArgs() {
-        assertEquals("1:8: first argument of [TRUNCATE('foo', 2)] must be [numeric], found value ['foo'] type [keyword]",
-            error("SELECT TRUNCATE('foo', 2)"));
-        assertEquals("1:8: second argument of [TRUNCATE(1.2, 'bar')] must be [integer], found value ['bar'] type [keyword]",
-            error("SELECT TRUNCATE(1.2, 'bar')"));
+        String functionName = randomFrom(Arrays.asList(Round.class, Truncate.class)).getSimpleName().toUpperCase(IsoLocale.ROOT);
+        assertEquals("1:8: first argument of [" + functionName + "('foo', 2)] must be [numeric], found value ['foo'] type [keyword]",
+            error("SELECT " + functionName + "('foo', 2)"));
+        assertEquals("1:8: second argument of [" + functionName + "(1.2, 'bar')] must be [integer], found value ['bar'] type [keyword]",
+            error("SELECT " + functionName + "(1.2, 'bar')"));
+        assertEquals("1:8: second argument of [" + functionName + "(1.2, 3.4)] must be [integer], found value [3.4] type [double]",
+            error("SELECT " + functionName + "(1.2, 3.4)"));
     }
 
     public void testInvalidTypeForBooleanFuntion_WithTwoArgs() {
@@ -623,9 +637,13 @@ public class VerifierErrorMessagesTests extends ESTestCase {
 
         assertEquals("1:8: second argument of [SUBSTRING('foo', 'bar', 3)] must be [integer], found value ['bar'] type [keyword]",
             error("SELECT SUBSTRING('foo', 'bar', 3)"));
+        assertEquals("1:8: second argument of [SUBSTRING('foo', 1.2, 3)] must be [integer], found value [1.2] type [double]",
+            error("SELECT SUBSTRING('foo', 1.2, 3)"));
 
         assertEquals("1:8: third argument of [SUBSTRING('foo', 2, 'bar')] must be [integer], found value ['bar'] type [keyword]",
             error("SELECT SUBSTRING('foo', 2, 'bar')"));
+        assertEquals("1:8: third argument of [SUBSTRING('foo', 2, 3.4)] must be [integer], found value [3.4] type [double]",
+            error("SELECT SUBSTRING('foo', 2, 3.4)"));
     }
 
     public void testInvalidTypeForFunction_WithFourArgs() {

+ 1 - 1
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateAddProcessorTests.java

@@ -67,7 +67,7 @@ public class DateAddProcessorTests extends AbstractSqlWireSerializingTestCase<Da
         siae = expectThrows(SqlIllegalArgumentException.class,
             () -> new DateAdd(Source.EMPTY,
                 l("days"), l("foo"), randomDatetimeLiteral(), randomZone()).makePipe().asProcessor().process(null));
-        assertEquals("An integer is required; received [foo]", siae.getMessage());
+        assertEquals("A number is required; received [foo]", siae.getMessage());
 
         siae = expectThrows(SqlIllegalArgumentException.class,
             () -> new DateAdd(Source.EMPTY,