SQL: Make INTERVAL millis optional (#36043)

Fractions of the second are not mandatory anymore inside INTERVAL
 declarations

Fix #36032

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 @@ public final class Intervals {
         private final List<TimeUnit> units;
         private final List<Token> tokens;
         private final String name;
+        private boolean optional = false;
 
         ParserBuilder(DataType dataType) {
             units = new ArrayList<>(10);
@@ -138,12 +140,17 @@ public final class Intervals {
 
         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 @@ public final class Intervals {
     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 @@ public final class Intervals {
             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 final class Intervals {
         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 final class Intervals {
                 .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 final class Intervals {
                 .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 final class Intervals {
                 .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());
     }
 

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java

@@ -89,6 +89,13 @@ public class IntervalsTests extends ESTestCase {
         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 class IntervalsTests extends ESTestCase {
         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 class IntervalsTests extends ESTestCase {
         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 class IntervalsTests extends ESTestCase {
     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 class IntervalsTests extends ESTestCase {
                 + "], 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));