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). (cherry picked from commit 0516b6eaf5eb98fa5bd087c3fece80139a6b118e)
This commit is contained in:
parent
5caa101345
commit
6589617a51
|
@ -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
|
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.
|
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="^,^"]
|
[cols="^,^"]
|
||||||
|===
|
|===
|
||||||
2+h|Datetime units to add/subtract
|
2+h|Datetime units to add/subtract
|
||||||
|
|
|
@ -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
|
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
|
selectAddWithComplexExpressions1
|
||||||
SELECT gender, birth_date, TIMESTAMPADD('months', CASE WHEN gender = 'M' THEN 10 WHEN gender = 'F' THEN -10 ELSE 100 END,
|
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;
|
birth_date + INTERVAL 10 month) AS dt FROM test_emp WHERE dt > '1954-07-01'::date ORDER BY emp_no LIMIT 10;
|
||||||
|
|
|
@ -59,14 +59,14 @@ public class DateAddProcessor extends ThreeArgsDateTimeProcessor {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (numberOfUnits instanceof Integer == false) {
|
if (numberOfUnits instanceof Number == false) {
|
||||||
throw new SqlIllegalArgumentException("An integer is required; received [{}]", numberOfUnits);
|
throw new SqlIllegalArgumentException("A number is required; received [{}]", numberOfUnits);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (timestamp instanceof ZonedDateTime == false) {
|
if (timestamp instanceof ZonedDateTime == false) {
|
||||||
throw new SqlIllegalArgumentException("A date/datetime is required; received [{}]", timestamp);
|
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());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -11,6 +11,10 @@ import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
|
||||||
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
|
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
|
||||||
import org.elasticsearch.xpack.sql.analysis.index.IndexResolverTests;
|
import org.elasticsearch.xpack.sql.analysis.index.IndexResolverTests;
|
||||||
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
|
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.Coalesce;
|
||||||
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest;
|
import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest;
|
||||||
import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull;
|
import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull;
|
||||||
|
@ -23,7 +27,9 @@ import org.elasticsearch.xpack.sql.type.DataType;
|
||||||
import org.elasticsearch.xpack.sql.type.EsField;
|
import org.elasticsearch.xpack.sql.type.EsField;
|
||||||
import org.elasticsearch.xpack.sql.type.TypesTests;
|
import org.elasticsearch.xpack.sql.type.TypesTests;
|
||||||
|
|
||||||
|
import java.util.Arrays;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
|
import java.util.Locale;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
||||||
import static java.util.Collections.emptyMap;
|
import static java.util.Collections.emptyMap;
|
||||||
|
@ -259,8 +265,8 @@ public class VerifierErrorMessagesTests extends ESTestCase {
|
||||||
public void testDateAddInvalidArgs() {
|
public void testDateAddInvalidArgs() {
|
||||||
assertEquals("1:8: first argument of [DATE_ADD(int, int, date)] must be [string], found value [int] type [integer]",
|
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"));
|
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] " +
|
assertEquals("1:8: second argument of [DATE_ADD(keyword, 1.2, date)] must be [integer], found value [1.2] " +
|
||||||
"type [keyword]", error("SELECT DATE_ADD(keyword, keyword, date) FROM test"));
|
"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] " +
|
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"));
|
"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, " +
|
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() {
|
public void testDateAddValidArgs() {
|
||||||
accept("SELECT DATE_ADD('weekday', 0, date) FROM test");
|
accept("SELECT DATE_ADD('weekday', 0, date) FROM test");
|
||||||
accept("SELECT DATE_ADD('dw', 20, date) FROM test");
|
accept("SELECT DATEADD('dw', 20, date) FROM test");
|
||||||
accept("SELECT DATE_ADD('years', -10, date) FROM test");
|
accept("SELECT TIMESTAMP_ADD('years', -10, date) FROM test");
|
||||||
accept("SELECT DATE_ADD('dayofyear', 123, date) FROM test");
|
accept("SELECT TIMESTAMPADD('dayofyear', 123, date) FROM test");
|
||||||
accept("SELECT DATE_ADD('dy', 30, date) FROM test");
|
accept("SELECT DATE_ADD('dy', 30, date) FROM test");
|
||||||
accept("SELECT DATE_ADD('ms', 1, date::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() {
|
public void testInvalidTypeForStringFunction_WithOneArgNumeric() {
|
||||||
assertEquals("1:8: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]",
|
String functionName = randomFrom(Arrays.asList(Char.class, Space.class)).getSimpleName().toUpperCase(Locale.ROOT);
|
||||||
error("SELECT CHAR('foo')"));
|
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() {
|
public void testInvalidTypeForNestedStringFunctions_WithOneArg() {
|
||||||
assertEquals("1:14: argument of [CHAR('foo')] must be [integer], found value ['foo'] type [keyword]",
|
assertEquals("1:15: argument of [SPACE('foo')] must be [integer], found value ['foo'] type [keyword]",
|
||||||
error("SELECT ASCII(CHAR('foo'))"));
|
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() {
|
public void testInvalidTypeForNumericFunction_WithOneArg() {
|
||||||
|
@ -581,10 +592,13 @@ public class VerifierErrorMessagesTests extends ESTestCase {
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testInvalidTypeForNumericFunction_WithTwoArgs() {
|
public void testInvalidTypeForNumericFunction_WithTwoArgs() {
|
||||||
assertEquals("1:8: first argument of [TRUNCATE('foo', 2)] must be [numeric], found value ['foo'] type [keyword]",
|
String functionName = randomFrom(Arrays.asList(Round.class, Truncate.class)).getSimpleName().toUpperCase(Locale.ROOT);
|
||||||
error("SELECT TRUNCATE('foo', 2)"));
|
assertEquals("1:8: first argument of [" + functionName + "('foo', 2)] must be [numeric], found value ['foo'] type [keyword]",
|
||||||
assertEquals("1:8: second argument of [TRUNCATE(1.2, 'bar')] must be [integer], found value ['bar'] type [keyword]",
|
error("SELECT " + functionName + "('foo', 2)"));
|
||||||
error("SELECT TRUNCATE(1.2, 'bar')"));
|
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() {
|
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]",
|
assertEquals("1:8: second argument of [SUBSTRING('foo', 'bar', 3)] must be [integer], found value ['bar'] type [keyword]",
|
||||||
error("SELECT SUBSTRING('foo', 'bar', 3)"));
|
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]",
|
assertEquals("1:8: third argument of [SUBSTRING('foo', 2, 'bar')] must be [integer], found value ['bar'] type [keyword]",
|
||||||
error("SELECT SUBSTRING('foo', 2, 'bar')"));
|
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() {
|
public void testInvalidTypeForFunction_WithFourArgs() {
|
||||||
|
|
|
@ -67,7 +67,7 @@ public class DateAddProcessorTests extends AbstractSqlWireSerializingTestCase<Da
|
||||||
siae = expectThrows(SqlIllegalArgumentException.class,
|
siae = expectThrows(SqlIllegalArgumentException.class,
|
||||||
() -> new DateAdd(Source.EMPTY,
|
() -> new DateAdd(Source.EMPTY,
|
||||||
l("days"), l("foo"), randomDatetimeLiteral(), randomZone()).makePipe().asProcessor().process(null));
|
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,
|
siae = expectThrows(SqlIllegalArgumentException.class,
|
||||||
() -> new DateAdd(Source.EMPTY,
|
() -> new DateAdd(Source.EMPTY,
|
||||||
|
|
Loading…
Reference in New Issue