Skip to content

Commit

Permalink
SQL: Make INTERVAL millis optional (#36043)
Browse files Browse the repository at this point in the history 
Fractions of the second are not mandatory anymore inside INTERVAL
 declarations

Fix #36032
  • Loading branch information
@costin
costin committed Nov 29, 2018
1 parent c305f9d commit 1d458e3
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
36 changes: 28 additions & 8 deletions ...ck/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/Intervals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -124,6 +125,7 @@ private static class ParserBuilder {
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);
Expand All @@ -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;
}

Expand All @@ -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]";
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());

Expand All @@ -357,15 +376,16 @@ 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());

PARSERS.put(DataType.INTERVAL_MINUTE_TO_SECOND, new ParserBuilder(DataType.INTERVAL_MINUTE_TO_SECOND)
.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());
}

Expand Down
42 changes: 36 additions & 6 deletions ...ugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/IntervalsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 1d458e3

Please sign in to comment.