From 5fb78cf63543a2ff1ed6616e52c95ee5a470cd87 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 29 Nov 2018 13:55:50 +0200 Subject: [PATCH] SQL: Make INTERVAL millis optional Fractions of the second are not mandatory anymore inside INTERVAL declarations Fix #36032 --- .../sql/expression/literal/Intervals.java | 36 ++++++++++++---- .../expression/literal/IntervalsTests.java | 42 ++++++++++++++++--- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/Intervals.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/Intervals.java index ad3b345e9c560..a64535e83b729 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/Intervals.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/Intervals.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.expression.literal; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.NamedWriteableRegistry.Entry; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; @@ -124,6 +125,7 @@ private static class ParserBuilder { private final List units; private final List tokens; private final String name; + private boolean optional = false; ParserBuilder(DataType dataType) { units = new ArrayList<>(10); @@ -138,12 +140,17 @@ ParserBuilder unit(TimeUnit unit) { ParserBuilder unit(TimeUnit unit, int maxValue) { units.add(unit); - tokens.add(new Token((char) 0, maxValue)); + tokens.add(new Token((char) 0, maxValue, optional)); return this; } ParserBuilder separator(char ch) { - tokens.add(new Token(ch, 0)); + tokens.add(new Token(ch, 0, optional)); + return this; + } + + ParserBuilder optional() { + optional = true; return this; } @@ -155,15 +162,17 @@ Parser build() { private static class Token { private final char ch; private final int maxValue; + private final boolean optional; - Token(char ch, int maxValue) { + Token(char ch, int maxValue, boolean optional) { this.ch = ch; this.maxValue = maxValue; + this.optional = optional; } @Override public String toString() { - return ch > 0 ? String.valueOf(ch) : "[numeric" + (maxValue > 0 ? " < " + maxValue + " " : "") + "]"; + return ch > 0 ? String.valueOf(ch) : "[numeric]"; } } @@ -203,6 +212,15 @@ TemporalAmount parse(Location source, String string) { for (Token token : tokens) { endToken = startToken; + if (startToken >= string.length()) { + // consumed the string, bail out + if (token.optional) { + break; + } + throw new ParsingException(source, invalidIntervalMessage(string) + ": incorrect format, expecting {}", + Strings.collectionToDelimitedString(tokens, "")); + } + // char token if (token.ch != 0) { char found = string.charAt(startToken); @@ -309,8 +327,8 @@ public static TemporalAmount negate(TemporalAmount interval) { PARSERS.put(DataType.INTERVAL_MINUTE, new ParserBuilder(DataType.INTERVAL_MINUTE).unit(TimeUnit.MINUTE).build()); PARSERS.put(DataType.INTERVAL_SECOND, new ParserBuilder(DataType.INTERVAL_SECOND) .unit(TimeUnit.SECOND) - .separator(DOT) - .unit(TimeUnit.MILLISECOND, MAX_MILLI) + .optional() + .separator(DOT).unit(TimeUnit.MILLISECOND, MAX_MILLI) .build()); // patterns @@ -342,6 +360,7 @@ public static TemporalAmount negate(TemporalAmount interval) { .unit(TimeUnit.MINUTE, MAX_MINUTE) .separator(COLON) .unit(TimeUnit.SECOND, MAX_SECOND) + .optional() .separator(DOT).unit(TimeUnit.MILLISECOND, MAX_MILLI) .build()); @@ -357,6 +376,7 @@ public static TemporalAmount negate(TemporalAmount interval) { .unit(TimeUnit.MINUTE, MAX_MINUTE) .separator(COLON) .unit(TimeUnit.SECOND, MAX_SECOND) + .optional() .separator(DOT).unit(TimeUnit.MILLISECOND, MAX_MILLI) .build()); @@ -364,8 +384,8 @@ public static TemporalAmount negate(TemporalAmount interval) { .unit(TimeUnit.MINUTE) .separator(COLON) .unit(TimeUnit.SECOND, MAX_SECOND) - .separator(DOT) - .unit(TimeUnit.MILLISECOND, MAX_MILLI) + .optional() + .separator(DOT).unit(TimeUnit.MILLISECOND, MAX_MILLI) .build()); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java index 521f7cb26e18f..bc84f3837ecca 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java @@ -89,6 +89,13 @@ public void testSecondInterval() throws Exception { assertEquals(maybeNegate(sign, Duration.ofSeconds(randomSeconds).plusMillis(randomMillis)), amount); } + public void testSecondNoMillisInterval() throws Exception { + int randomSeconds = randomNonNegativeInt(); + String value = format(Locale.ROOT, "%s%d", sign, randomSeconds); + TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_SECOND); + assertEquals(maybeNegate(sign, Duration.ofSeconds(randomSeconds)), amount); + } + public void testYearToMonth() throws Exception { int randomYear = randomNonNegativeInt(); int randomMonth = randomInt(11); @@ -119,9 +126,12 @@ public void testDayToSecond() throws Exception { int randomHour = randomInt(23); int randomMinute = randomInt(59); int randomSecond = randomInt(59); - int randomMilli = randomInt(999999999); - String value = format(Locale.ROOT, "%s%d %d:%d:%d.%d", sign, randomDay, randomHour, randomMinute, randomSecond, randomMilli); + boolean withMillis = randomBoolean(); + int randomMilli = withMillis ? randomInt(999999999) : 0; + String millisString = withMillis ? "." + randomMilli : ""; + + String value = format(Locale.ROOT, "%s%d %d:%d:%d%s", sign, randomDay, randomHour, randomMinute, randomSecond, millisString); TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_DAY_TO_SECOND); assertEquals(maybeNegate(sign, Duration.ofDays(randomDay).plusHours(randomHour).plusMinutes(randomMinute) .plusSeconds(randomSecond).plusMillis(randomMilli)), amount); @@ -139,9 +149,12 @@ public void testHourToSecond() throws Exception { int randomHour = randomNonNegativeInt(); int randomMinute = randomInt(59); int randomSecond = randomInt(59); - int randomMilli = randomInt(999999999); - String value = format(Locale.ROOT, "%s%d:%d:%d.%d", sign, randomHour, randomMinute, randomSecond, randomMilli); + boolean withMillis = randomBoolean(); + int randomMilli = withMillis ? randomInt(999999999) : 0; + String millisString = withMillis ? "." + randomMilli : ""; + + String value = format(Locale.ROOT, "%s%d:%d:%d%s", sign, randomHour, randomMinute, randomSecond, millisString); TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_HOUR_TO_SECOND); assertEquals(maybeNegate(sign, Duration.ofHours(randomHour).plusMinutes(randomMinute).plusSeconds(randomSecond).plusMillis(randomMilli)), amount); @@ -150,9 +163,12 @@ public void testHourToSecond() throws Exception { public void testMinuteToSecond() throws Exception { int randomMinute = randomNonNegativeInt(); int randomSecond = randomInt(59); - int randomMilli = randomInt(999999999); - String value = format(Locale.ROOT, "%s%d:%d.%d", sign, randomMinute, randomSecond, randomMilli); + boolean withMillis = randomBoolean(); + int randomMilli = withMillis ? randomInt(999999999) : 0; + String millisString = withMillis ? "." + randomMilli : ""; + + String value = format(Locale.ROOT, "%s%d:%d%s", sign, randomMinute, randomSecond, millisString); TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_MINUTE_TO_SECOND); assertEquals(maybeNegate(sign, Duration.ofMinutes(randomMinute).plusSeconds(randomSecond).plusMillis(randomMilli)), amount); } @@ -187,6 +203,20 @@ public void testDayToMinuteTooBig() throws Exception { + "], expected a positive number up to [23]", pe.getMessage()); } + public void testIncompleteYearToMonthInterval() throws Exception { + String value = "123-"; + ParsingException pe = expectThrows(ParsingException.class, () -> parseInterval(EMPTY, value, INTERVAL_YEAR_TO_MONTH)); + assertEquals("line -1:0: Invalid [INTERVAL YEAR TO MONTH] value [123-]: incorrect format, expecting [numeric]-[numeric]", + pe.getMessage()); + } + + public void testIncompleteDayToHourInterval() throws Exception { + String value = "123 23:"; + ParsingException pe = expectThrows(ParsingException.class, () -> parseInterval(EMPTY, value, INTERVAL_DAY_TO_HOUR)); + assertEquals("line -1:0: Invalid [INTERVAL DAY TO HOUR] value [123 23:]: unexpected trailing characters found [:]", + pe.getMessage()); + } + public void testExtraCharLeading() throws Exception { String value = "a123"; ParsingException pe = expectThrows(ParsingException.class, () -> parseInterval(EMPTY, value, INTERVAL_YEAR));