Improve messages for INTERVAL literal semantic errors #23002
Conversation
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
I'll take a look this Wednesday. |
| @@ -191,24 +191,24 @@ 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); | |||
There was a problem hiding this comment.
Move the typo fix to a separate commit.
There was a problem hiding this comment.
Moved to a dedicated commit.
takezoe
force-pushed
the
improve-interval-error
branch
from
162762f to
8dc209a
Compare
|
Would we allow cases where This seems valid syntactically and Trino treats this in the same way as |
Doesn't seem to be true. See #23157 (comment). From spec:
|
|
Ah, it is a part of syntax rules so syntactically invalid. Will update this PR to raise an error in that case. Thanks. |
takezoe
force-pushed
the
improve-interval-error
branch
2 times, most recently
from
fb002ab to
2ac032e
Compare
|
@hashhar Updated the PR to raise an error when FROM and TO are the same. Could you take a look? |
|
@hashhar ping |
|
Sorry for the delay @takezoe, I'll do a final pass tomorrow (I skimmed it once more right now and don't see anything wrong) and merge. |
takezoe
force-pushed
the
improve-interval-error
branch
from
6eaca40 to
0733431
Compare
|
@hashhar Thank you! All commits except one for typo fix can be squashed. |
hashhar
force-pushed
the
improve-interval-error
branch
from
5692073 to
6a6a3ac
Compare
|
Unrelated CI failure. Merging. Thanks @takezoe and sorry for the wait. |
Description
When an INTERVAL literal is invalid,
ExpressionAnalyzerignores an original error and always report as'xxxx' is not a valid INTERVAL literalwhich is confusing in cases like below:INTERVAL '12-10' YEAR TO DAYIn this case,
YEAR TO DAYis invalid in the first place but still reported as'12-10' is not a valid INTERVAL literal.It would be better to keep the original error message for better error reporting.
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: