From 3166b8ef3222c706cbae45312e626c592a74c470 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 8 Feb 2020 16:03:09 +0100 Subject: [PATCH 1/5] SQL: Enhance timestamp escaped literal parsing Allow also whitespace ` ` (together with `T`) as a separator between date and time parts of the timestamp string. E.g.: ``` 2020-02-08 12.10.45 ``` or ``` 2020-02-08T12.10.45 ``` Fixes: #46049 --- .../xpack/sql/util/DateUtils.java | 22 ++++++++++++++----- .../sql/parser/EscapedFunctionsTests.java | 10 ++++++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java index 448b6bc537602..56d6daeaddab4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java @@ -20,6 +20,7 @@ import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; +import java.time.format.DateTimeParseException; import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; @@ -32,11 +33,16 @@ public final class DateUtils { public static final LocalDate EPOCH = LocalDate.of(1970, 1, 1); public static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000L; - private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER = new DateTimeFormatterBuilder() - .append(ISO_LOCAL_DATE) - .appendLiteral(" ") - .append(ISO_LOCAL_TIME) - .toFormatter().withZone(UTC); + private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER_WHITESPACE = new DateTimeFormatterBuilder() + .append(ISO_LOCAL_DATE) + .appendLiteral(" ") + .append(ISO_LOCAL_TIME) + .toFormatter().withZone(UTC); + private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER_T_LITERAL = new DateTimeFormatterBuilder() + .append(ISO_LOCAL_DATE) + .appendLiteral("T") + .append(ISO_LOCAL_TIME) + .toFormatter().withZone(UTC); private static final DateFormatter UTC_DATE_TIME_FORMATTER = DateFormatter.forPattern("date_optional_time").withZone(UTC); private static final int DEFAULT_PRECISION_FOR_CURRENT_FUNCTIONS = 3; @@ -105,7 +111,11 @@ public static ZonedDateTime asDateTime(String dateFormat) { } public static ZonedDateTime ofEscapedLiteral(String dateFormat) { - return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER.withZone(UTC)); + try { + return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_T_LITERAL.withZone(UTC)); + } catch (DateTimeParseException e) { + return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_WHITESPACE.withZone(UTC)); + } } public static String toString(ZonedDateTime dateTime) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index 2852619edb004..f69e00cf83488 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -209,14 +209,18 @@ public void testTimeLiteralValidation() { } public void testTimestampLiteral() { - Literal l = timestampLiteral("2012-01-01 10:01:02.3456"); + String fractionalSecs = randomFrom("", ".1", ".12", ".123", ".1234", ".12345", ".123456", + ".1234567", ".12345678", ".123456789"); + Literal l = timestampLiteral("2012-01-01 10:01:02" + fractionalSecs); + assertThat(l.dataType(), is(DATETIME)); + l = timestampLiteral("2012-01-01T10:01:02" + fractionalSecs); assertThat(l.dataType(), is(DATETIME)); } public void testTimestampLiteralValidation() { - ParsingException ex = expectThrows(ParsingException.class, () -> timestampLiteral("2012-01-01T10:01:02.3456")); + ParsingException ex = expectThrows(ParsingException.class, () -> timestampLiteral("2012-01-01_AB 10:01:02.3456")); assertEquals( - "line 1:2: Invalid timestamp received; Text '2012-01-01T10:01:02.3456' could not be parsed at index 10", + "line 1:2: Invalid timestamp received; Text '2012-01-01_AB 10:01:02.3456' could not be parsed at index 10", ex.getMessage()); } From 3e294010106af2c2eaab1f01bf7cf9af91b5e53e Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 8 Feb 2020 19:08:31 +0100 Subject: [PATCH 2/5] enhance escaped time literal tests --- .../sql/parser/EscapedFunctionsTests.java | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index f69e00cf83488..4deb0f29b6b08 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -62,6 +62,21 @@ private Literal timestampLiteral(String date) { return (Literal) exp; } + private String buildSecsAndFractional() { + boolean hasSecs = randomBoolean(); + String secs = ""; + if (hasSecs) { + secs = ":55"; + } + + String fractionalSecs = ""; + if (hasSecs) { + fractionalSecs = randomFrom("", ".1", ".12", ".123", ".1234", ".12345", ".123456", + ".1234567", ".12345678", ".123456789"); + } + return secs + fractionalSecs; + } + private Literal guidLiteral(String guid) { Expression exp = parser.createExpression(buildExpression("guid", "'%s'", guid)); assertThat(exp, instanceOf(Expression.class)); @@ -197,7 +212,7 @@ public void testDateLiteralValidation() { } public void testTimeLiteral() { - Literal l = timeLiteral("12:23:56"); + Literal l = timeLiteral("12:23" + buildSecsAndFractional()); assertThat(l.dataType(), is(TIME)); } @@ -209,11 +224,10 @@ public void testTimeLiteralValidation() { } public void testTimestampLiteral() { - String fractionalSecs = randomFrom("", ".1", ".12", ".123", ".1234", ".12345", ".123456", - ".1234567", ".12345678", ".123456789"); - Literal l = timestampLiteral("2012-01-01 10:01:02" + fractionalSecs); + + Literal l = timestampLiteral("2012-01-01 10:01" + buildSecsAndFractional()); assertThat(l.dataType(), is(DATETIME)); - l = timestampLiteral("2012-01-01T10:01:02" + fractionalSecs); + l = timestampLiteral("2012-01-01T10:01" + buildSecsAndFractional()); assertThat(l.dataType(), is(DATETIME)); } From 439c3504dc522c91345584545ad1362f8342a16e Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 8 Feb 2020 19:46:32 +0100 Subject: [PATCH 3/5] Address comments - enhance tests --- .../xpack/sql/util/DateUtils.java | 10 +++---- .../sql/parser/EscapedFunctionsTests.java | 30 ++++++++++++++++--- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java index 56d6daeaddab4..8bb30245a4656 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java @@ -20,7 +20,6 @@ import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeFormatterBuilder; -import java.time.format.DateTimeParseException; import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; @@ -35,12 +34,12 @@ public final class DateUtils { private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER_WHITESPACE = new DateTimeFormatterBuilder() .append(ISO_LOCAL_DATE) - .appendLiteral(" ") + .appendLiteral(' ') .append(ISO_LOCAL_TIME) .toFormatter().withZone(UTC); private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER_T_LITERAL = new DateTimeFormatterBuilder() .append(ISO_LOCAL_DATE) - .appendLiteral("T") + .appendLiteral('T') .append(ISO_LOCAL_TIME) .toFormatter().withZone(UTC); @@ -111,9 +110,10 @@ public static ZonedDateTime asDateTime(String dateFormat) { } public static ZonedDateTime ofEscapedLiteral(String dateFormat) { - try { + int separatorIdx = dateFormat.lastIndexOf('-') + 3; + if (dateFormat.charAt(separatorIdx) == 'T') { return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_T_LITERAL.withZone(UTC)); - } catch (DateTimeParseException e) { + } else { return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_WHITESPACE.withZone(UTC)); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index 4deb0f29b6b08..1fb74447f0fec 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -62,6 +62,25 @@ private Literal timestampLiteral(String date) { return (Literal) exp; } + private String buildDate() { + StringBuilder sb = new StringBuilder(); + int length = randomIntBetween(4, 9); + + if (randomBoolean()) { + sb.append('-'); + } else { + if (length > 4) { + sb.append('-'); + } + } + + for (int i = 1; i <= length; i++) { + sb.append(i); + } + sb.append("-05-10"); + return sb.toString(); + } + private String buildSecsAndFractional() { boolean hasSecs = randomBoolean(); String secs = ""; @@ -200,7 +219,7 @@ public void testFunctionWithFunctionWithArgAndParams() { } public void testDateLiteral() { - Literal l = dateLiteral("2012-01-01"); + Literal l = dateLiteral(buildDate()); assertThat(l.dataType(), is(DATE)); } @@ -224,10 +243,9 @@ public void testTimeLiteralValidation() { } public void testTimestampLiteral() { - - Literal l = timestampLiteral("2012-01-01 10:01" + buildSecsAndFractional()); + Literal l = timestampLiteral(buildDate() + " 10:20" + buildSecsAndFractional()); assertThat(l.dataType(), is(DATETIME)); - l = timestampLiteral("2012-01-01T10:01" + buildSecsAndFractional()); + l = timestampLiteral(buildDate() + "T11:22" + buildSecsAndFractional()); assertThat(l.dataType(), is(DATETIME)); } @@ -236,6 +254,10 @@ public void testTimestampLiteralValidation() { assertEquals( "line 1:2: Invalid timestamp received; Text '2012-01-01_AB 10:01:02.3456' could not be parsed at index 10", ex.getMessage()); + ex = expectThrows(ParsingException.class, () -> timestampLiteral("20120101_AB 10:01:02.3456")); + assertEquals( + "line 1:2: Invalid timestamp received; Text '20120101_AB 10:01:02.3456' could not be parsed at index 0", + ex.getMessage()); } public void testGUID() { From cb520d7126b0a878ccc5f67c2f9e3e112f1df0fb Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sat, 8 Feb 2020 20:05:58 +0100 Subject: [PATCH 4/5] fix index out of bounds --- .../org/elasticsearch/xpack/sql/util/DateUtils.java | 3 ++- .../xpack/sql/parser/EscapedFunctionsTests.java | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java index 8bb30245a4656..31c79f7a1bc6d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java @@ -111,7 +111,8 @@ public static ZonedDateTime asDateTime(String dateFormat) { public static ZonedDateTime ofEscapedLiteral(String dateFormat) { int separatorIdx = dateFormat.lastIndexOf('-') + 3; - if (dateFormat.charAt(separatorIdx) == 'T') { + // Avoid index out of bounds - it will lead to DateTimeParseException anyways + if (separatorIdx >= dateFormat.length() || dateFormat.charAt(separatorIdx) == 'T') { return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_T_LITERAL.withZone(UTC)); } else { return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER_WHITESPACE.withZone(UTC)); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index 1fb74447f0fec..5979f1658db67 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -250,14 +250,21 @@ public void testTimestampLiteral() { } public void testTimestampLiteralValidation() { - ParsingException ex = expectThrows(ParsingException.class, () -> timestampLiteral("2012-01-01_AB 10:01:02.3456")); + String date = buildDate(); + ParsingException ex = expectThrows(ParsingException.class, () -> timestampLiteral(date+ "_AB 10:01:02.3456")); assertEquals( - "line 1:2: Invalid timestamp received; Text '2012-01-01_AB 10:01:02.3456' could not be parsed at index 10", + "line 1:2: Invalid timestamp received; Text '" + date + "_AB 10:01:02.3456' could not be parsed at index " + + date.length(), ex.getMessage()); ex = expectThrows(ParsingException.class, () -> timestampLiteral("20120101_AB 10:01:02.3456")); assertEquals( "line 1:2: Invalid timestamp received; Text '20120101_AB 10:01:02.3456' could not be parsed at index 0", ex.getMessage()); + + ex = expectThrows(ParsingException.class, () -> timestampLiteral(date)); + assertEquals( + "line 1:2: Invalid timestamp received; Text '" + date + "' could not be parsed at index " + date.length(), + ex.getMessage()); } public void testGUID() { From 406433f045a13db7a21bcc22bf33ead67d7be0c9 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sun, 9 Feb 2020 19:37:44 +0100 Subject: [PATCH 5/5] address comments --- .../xpack/sql/parser/EscapedFunctionsTests.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index 5979f1658db67..93395f9dbd338 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -82,18 +82,11 @@ private String buildDate() { } private String buildSecsAndFractional() { - boolean hasSecs = randomBoolean(); - String secs = ""; - if (hasSecs) { - secs = ":55"; - } - - String fractionalSecs = ""; - if (hasSecs) { - fractionalSecs = randomFrom("", ".1", ".12", ".123", ".1234", ".12345", ".123456", + if (randomBoolean()) { + return ":55" + randomFrom("", ".1", ".12", ".123", ".1234", ".12345", ".123456", ".1234567", ".12345678", ".123456789"); } - return secs + fractionalSecs; + return ""; } private Literal guidLiteral(String guid) {