Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve messages for INTERVAL literal semantic errors #23002

Merged
merged 2 commits into from
Nov 27, 2024
+155 −56
Merged

Improve messages for INTERVAL literal semantic errors #23002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37b0207
Improve messages for INTERVAL literal semantic errors
takezoe Aug 11, 2024
6a6a3ac
Fix typo in DateTimeUtils
takezoe Sep 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,9 @@ protected Type visitIntervalLiteral(IntervalLiteral node, Context context)
try {
literalInterpreter.evaluate(node, type);
}
catch (TrinoException e) {
throw new TrinoException(e::getErrorCode, extractLocation(node), e.getMessage(), e);
}
catch (RuntimeException e) {
throw semanticException(INVALID_LITERAL, node, e, "'%s' is not a valid INTERVAL literal", node.getValue());
}
Expand Down
93 changes: 47 additions & 46 deletions core/trino-main/src/main/java/io/trino/util/DateTimeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import java.util.stream.Stream;

import static com.google.common.base.Preconditions.checkArgument;
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
import static io.trino.spi.StandardErrorCode.INVALID_LITERAL;
import static io.trino.sql.tree.IntervalLiteral.IntervalField;
import static io.trino.util.DateTimeZoneIndex.getChronology;
import static io.trino.util.DateTimeZoneIndex.packDateTimeWithZone;
Expand Down Expand Up @@ -191,69 +191,67 @@ public static long convertToTimestampWithTimeZone(TimeZoneKey timeZoneKey, Strin
private static final int SECOND_FIELD = 6;
private static final int MILLIS_FIELD = 7;

private static final PeriodFormatter INTERVAL_DAY_SECOND_FORMATTER = cretePeriodFormatter(IntervalField.DAY, IntervalField.SECOND);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the typo fix to a separate commit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to a dedicated commit.

private static final PeriodFormatter INTERVAL_DAY_MINUTE_FORMATTER = cretePeriodFormatter(IntervalField.DAY, IntervalField.MINUTE);
private static final PeriodFormatter INTERVAL_DAY_HOUR_FORMATTER = cretePeriodFormatter(IntervalField.DAY, IntervalField.HOUR);
private static final PeriodFormatter INTERVAL_DAY_FORMATTER = cretePeriodFormatter(IntervalField.DAY, IntervalField.DAY);
private static final PeriodFormatter INTERVAL_DAY_SECOND_FORMATTER = createPeriodFormatter(IntervalField.DAY, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_DAY_MINUTE_FORMATTER = createPeriodFormatter(IntervalField.DAY, IntervalField.MINUTE);
private static final PeriodFormatter INTERVAL_DAY_HOUR_FORMATTER = createPeriodFormatter(IntervalField.DAY, IntervalField.HOUR);
private static final PeriodFormatter INTERVAL_DAY_FORMATTER = createPeriodFormatter(IntervalField.DAY, IntervalField.DAY);

private static final PeriodFormatter INTERVAL_HOUR_SECOND_FORMATTER = cretePeriodFormatter(IntervalField.HOUR, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_HOUR_MINUTE_FORMATTER = cretePeriodFormatter(IntervalField.HOUR, IntervalField.MINUTE);
private static final PeriodFormatter INTERVAL_HOUR_FORMATTER = cretePeriodFormatter(IntervalField.HOUR, IntervalField.HOUR);
private static final PeriodFormatter INTERVAL_HOUR_SECOND_FORMATTER = createPeriodFormatter(IntervalField.HOUR, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_HOUR_MINUTE_FORMATTER = createPeriodFormatter(IntervalField.HOUR, IntervalField.MINUTE);
private static final PeriodFormatter INTERVAL_HOUR_FORMATTER = createPeriodFormatter(IntervalField.HOUR, IntervalField.HOUR);

private static final PeriodFormatter INTERVAL_MINUTE_SECOND_FORMATTER = cretePeriodFormatter(IntervalField.MINUTE, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_MINUTE_FORMATTER = cretePeriodFormatter(IntervalField.MINUTE, IntervalField.MINUTE);
private static final PeriodFormatter INTERVAL_MINUTE_SECOND_FORMATTER = createPeriodFormatter(IntervalField.MINUTE, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_MINUTE_FORMATTER = createPeriodFormatter(IntervalField.MINUTE, IntervalField.MINUTE);

private static final PeriodFormatter INTERVAL_SECOND_FORMATTER = cretePeriodFormatter(IntervalField.SECOND, IntervalField.SECOND);
private static final PeriodFormatter INTERVAL_SECOND_FORMATTER = createPeriodFormatter(IntervalField.SECOND, IntervalField.SECOND);

private static final PeriodFormatter INTERVAL_YEAR_MONTH_FORMATTER = cretePeriodFormatter(IntervalField.YEAR, IntervalField.MONTH);
private static final PeriodFormatter INTERVAL_YEAR_FORMATTER = cretePeriodFormatter(IntervalField.YEAR, IntervalField.YEAR);
private static final PeriodFormatter INTERVAL_YEAR_MONTH_FORMATTER = createPeriodFormatter(IntervalField.YEAR, IntervalField.MONTH);
private static final PeriodFormatter INTERVAL_YEAR_FORMATTER = createPeriodFormatter(IntervalField.YEAR, IntervalField.YEAR);

private static final PeriodFormatter INTERVAL_MONTH_FORMATTER = cretePeriodFormatter(IntervalField.MONTH, IntervalField.MONTH);
private static final PeriodFormatter INTERVAL_MONTH_FORMATTER = createPeriodFormatter(IntervalField.MONTH, IntervalField.MONTH);

public static long parseDayTimeInterval(String value, IntervalField startField, Optional<IntervalField> endField)
{
IntervalField end = endField.orElse(startField);
hashhar marked this conversation as resolved.
Show resolved Hide resolved

try {
if (startField == IntervalField.DAY && end == IntervalField.SECOND) {
if (startField == IntervalField.DAY && endField.isEmpty()) {
return parsePeriodMillis(INTERVAL_DAY_FORMATTER, value);
}
if (startField == IntervalField.DAY && endField.get() == IntervalField.SECOND) {
return parsePeriodMillis(INTERVAL_DAY_SECOND_FORMATTER, value);
}
if (startField == IntervalField.DAY && end == IntervalField.MINUTE) {
if (startField == IntervalField.DAY && endField.get() == IntervalField.MINUTE) {
return parsePeriodMillis(INTERVAL_DAY_MINUTE_FORMATTER, value);
}
if (startField == IntervalField.DAY && end == IntervalField.HOUR) {
if (startField == IntervalField.DAY && endField.get() == IntervalField.HOUR) {
return parsePeriodMillis(INTERVAL_DAY_HOUR_FORMATTER, value);
}
if (startField == IntervalField.DAY && end == IntervalField.DAY) {
return parsePeriodMillis(INTERVAL_DAY_FORMATTER, value);
}

if (startField == IntervalField.HOUR && end == IntervalField.SECOND) {
if (startField == IntervalField.HOUR && endField.isEmpty()) {
return parsePeriodMillis(INTERVAL_HOUR_FORMATTER, value);
}
if (startField == IntervalField.HOUR && endField.get() == IntervalField.SECOND) {
return parsePeriodMillis(INTERVAL_HOUR_SECOND_FORMATTER, value);
}
if (startField == IntervalField.HOUR && end == IntervalField.MINUTE) {
if (startField == IntervalField.HOUR && endField.get() == IntervalField.MINUTE) {
return parsePeriodMillis(INTERVAL_HOUR_MINUTE_FORMATTER, value);
}
if (startField == IntervalField.HOUR && end == IntervalField.HOUR) {
return parsePeriodMillis(INTERVAL_HOUR_FORMATTER, value);
}

if (startField == IntervalField.MINUTE && end == IntervalField.SECOND) {
return parsePeriodMillis(INTERVAL_MINUTE_SECOND_FORMATTER, value);
}
if (startField == IntervalField.MINUTE && end == IntervalField.MINUTE) {
if (startField == IntervalField.MINUTE && endField.isEmpty()) {
return parsePeriodMillis(INTERVAL_MINUTE_FORMATTER, value);
}
if (startField == IntervalField.MINUTE && endField.get() == IntervalField.SECOND) {
return parsePeriodMillis(INTERVAL_MINUTE_SECOND_FORMATTER, value);
}

if (startField == IntervalField.SECOND && end == IntervalField.SECOND) {
if (startField == IntervalField.SECOND && endField.isEmpty()) {
return parsePeriodMillis(INTERVAL_SECOND_FORMATTER, value);
}
}
catch (IllegalArgumentException e) {
throw invalidInterval(e, value, startField, end);
throw invalidInterval(e, value, startField, endField.orElse(startField));
}

throw new IllegalArgumentException("Invalid day second interval qualifier: " + startField + " to " + end);
throw invalidQualifier(startField, endField.orElse(startField));
}

private static long parsePeriodMillis(PeriodFormatter periodFormatter, String value)
Expand All @@ -269,25 +267,23 @@ private static long parsePeriodMillis(PeriodFormatter periodFormatter, String va

public static long parseYearMonthInterval(String value, IntervalField startField, Optional<IntervalField> endField)
{
IntervalField end = endField.orElse(startField);

try {
if (startField == IntervalField.YEAR && end == IntervalField.MONTH) {
return parsePeriodMonths(value, INTERVAL_YEAR_MONTH_FORMATTER);
}
if (startField == IntervalField.YEAR && end == IntervalField.YEAR) {
if (startField == IntervalField.YEAR && endField.isEmpty()) {
return parsePeriodMonths(value, INTERVAL_YEAR_FORMATTER);
}
if (startField == IntervalField.YEAR && endField.get() == IntervalField.MONTH) {
return parsePeriodMonths(value, INTERVAL_YEAR_MONTH_FORMATTER);
}

if (startField == IntervalField.MONTH && end == IntervalField.MONTH) {
if (startField == IntervalField.MONTH && endField.isEmpty()) {
return parsePeriodMonths(value, INTERVAL_MONTH_FORMATTER);
}
}
catch (IllegalArgumentException e) {
throw invalidInterval(e, value, startField, end);
throw invalidInterval(e, value, startField, endField.orElse(startField));
}

throw new IllegalArgumentException("Invalid year month interval qualifier: " + startField + " to " + end);
throw invalidQualifier(startField, endField.orElse(startField));
}

private static long parsePeriodMonths(String value, PeriodFormatter periodFormatter)
Expand Down Expand Up @@ -325,10 +321,15 @@ private static TrinoException invalidInterval(Throwable throwable, String value,
else {
message = format("Invalid INTERVAL %s TO %s value: %s", startField, endField, value);
}
return new TrinoException(INVALID_FUNCTION_ARGUMENT, message, throwable);
return new TrinoException(INVALID_LITERAL, message, throwable);
}

private static TrinoException invalidQualifier(IntervalField startField, IntervalField endField)
{
throw new TrinoException(INVALID_LITERAL, "Invalid interval qualifier: " + startField + " TO " + endField);
}

private static PeriodFormatter cretePeriodFormatter(IntervalField startField, IntervalField endField)
private static PeriodFormatter createPeriodFormatter(IntervalField startField, IntervalField endField)
{
if (endField == null) {
endField = startField;
Expand All @@ -351,7 +352,7 @@ private static PeriodFormatter cretePeriodFormatter(IntervalField startField, In
builder.appendMonths();
parsers.add(builder.toParser());
if (endField != IntervalField.MONTH) {
throw new IllegalArgumentException("Invalid interval qualifier: " + startField + " to " + endField);
throw invalidQualifier(startField, endField);
}
break;

Expand Down
69 changes: 64 additions & 5 deletions core/trino-main/src/test/java/io/trino/operator/scalar/interval/TestIntervalDayTime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,20 +138,79 @@ public void testLiterals()
assertThat(assertions.expression("INTERVAL '32' SECOND"))
.isEqualTo(interval(0, 0, 0, 32, 0));

// Invalid literals
assertThatThrownBy(assertions.expression("INTERVAL '12X' DAY")::evaluate)
.hasMessage("line 1:12: '12X' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL DAY value: 12X");

assertThatThrownBy(assertions.expression("INTERVAL '12 10' DAY")::evaluate)
.hasMessage("line 1:12: '12 10' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL DAY value: 12 10");

assertThatThrownBy(assertions.expression("INTERVAL '12 X' DAY TO HOUR")::evaluate)
.hasMessage("line 1:12: '12 X' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL DAY TO HOUR value: 12 X");

assertThatThrownBy(assertions.expression("INTERVAL '12 -10' DAY TO HOUR")::evaluate)
.hasMessage("line 1:12: '12 -10' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL DAY TO HOUR value: 12 -10");

assertThatThrownBy(assertions.expression("INTERVAL '--12 -10' DAY TO HOUR")::evaluate)
.hasMessage("line 1:12: '--12 -10' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL DAY TO HOUR value: --12 -10");

// Invalid qualifiers (DAY TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '12' DAY TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: DAY TO DAY");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' DAY TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: DAY TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' DAY TO MONTH")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: DAY TO MONTH");

// Invalid qualifiers (HOUR TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '10' HOUR TO HOUR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: HOUR TO HOUR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' HOUR TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: HOUR TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' HOUR TO MONTH")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: HOUR TO MONTH");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' HOUR TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: HOUR TO DAY");

// Invalid qualifiers (MINUTE TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '45' MINUTE TO MINUTE")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MINUTE TO MINUTE");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' MINUTE TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MINUTE TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' MINUTE TO MONTH")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MINUTE TO MONTH");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' MINUTE TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MINUTE TO DAY");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' MINUTE TO HOUR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MINUTE TO HOUR");

// Invalid qualifiers (SECOND TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '32' SECOND TO SECOND")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO SECOND");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' SECOND TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' SECOND TO MONTH")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO MONTH");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' SECOND TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO DAY");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' SECOND TO HOUR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO HOUR");

assertThatThrownBy(assertions.expression("INTERVAL '12-10' SECOND TO MINUTE")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: SECOND TO MINUTE");
}

private static SqlIntervalDayTime interval(int day, int hour, int minute, int second, int milliseconds)
Expand Down
46 changes: 41 additions & 5 deletions core/trino-main/src/test/java/io/trino/operator/scalar/interval/TestIntervalYearMonth.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,56 @@ public void testLiterals()
assertThat(assertions.expression("INTERVAL '32767-32767' YEAR TO MONTH"))
.isEqualTo(interval(32767, 32767));

// Invalid literals
assertThatThrownBy(assertions.expression("INTERVAL '124X' YEAR")::evaluate)
.hasMessage("line 1:12: '124X' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL YEAR value: 124X");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' YEAR")::evaluate)
.hasMessage("line 1:12: '124-30' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL YEAR value: 124-30");

assertThatThrownBy(assertions.expression("INTERVAL '124-X' YEAR TO MONTH")::evaluate)
.hasMessage("line 1:12: '124-X' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL YEAR TO MONTH value: 124-X");

assertThatThrownBy(assertions.expression("INTERVAL '124--30' YEAR TO MONTH")::evaluate)
.hasMessage("line 1:12: '124--30' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL YEAR TO MONTH value: 124--30");

assertThatThrownBy(assertions.expression("INTERVAL '--124--30' YEAR TO MONTH")::evaluate)
.hasMessage("line 1:12: '--124--30' is not a valid INTERVAL literal");
.hasMessage("line 1:12: Invalid INTERVAL YEAR TO MONTH value: --124--30");

// Invalid qualifiers (YEAR TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '32767' YEAR TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: YEAR TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' YEAR TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: YEAR TO DAY");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' YEAR TO HOUR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: YEAR TO HOUR");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' YEAR TO MINUTE")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: YEAR TO MINUTE");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' YEAR TO SECOND")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: YEAR TO SECOND");

// Invalid qualifiers (MONTH TO xxx)
assertThatThrownBy(assertions.expression("INTERVAL '30' MONTH TO MONTH")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO MONTH");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' MONTH TO YEAR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO YEAR");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' MONTH TO DAY")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO DAY");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' MONTH TO HOUR")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO HOUR");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' MONTH TO MINUTE")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO MINUTE");

assertThatThrownBy(assertions.expression("INTERVAL '124-30' MONTH TO SECOND")::evaluate)
.hasMessage("line 1:12: Invalid interval qualifier: MONTH TO SECOND");
}

private static SqlIntervalYearMonth interval(int year, int month)
Expand Down
Loading