Minor cleanup in parsing error handler #1042
Conversation
| @@ -182,6 +182,12 @@ public Analyzer( | |||
| int tokenIndex = current.tokenIndex; | |||
| CallerContext caller = current.caller; | |||
|
|
|||
| while (tokenIndex < stream.size() && stream.get(tokenIndex).getType() == SqlBaseParser.WS) { | |||
There was a problem hiding this comment.
tokenIndex < stream.size()is redundantSqlBaseParser.WSshould be provided as configuration
There was a problem hiding this comment.
Do this instead: stream.get(tokenIndex).getChannel() == Token.HIDDEN_CHANNEL
presto-parser/src/main/java/io/prestosql/sql/parser/ErrorHandler.java
Outdated
Show resolved
Hide resolved
findepi
force-pushed
the
error-handler
branch
2 times, most recently
from
cb6af51
to
d6bc2ca
Compare
| @@ -182,6 +182,12 @@ public Analyzer( | |||
| int tokenIndex = current.tokenIndex; | |||
| CallerContext caller = current.caller; | |||
|
|
|||
| while (stream.get(tokenIndex).getChannel() == Token.HIDDEN_CHANNEL) { | |||
There was a problem hiding this comment.
I think this will fail if the trailing tokens are all whitespace -- it will run out of bounds
There was a problem hiding this comment.
I think EOF is always the last token and it's DEFAULT_CHANNEL (not HIDDEN_CHANNEL), so I don't need explicit bound check.
(Previously the code depended on this too, bounds were never checked explicitly.)
There was a problem hiding this comment.
I added one more test case: "CREATE TABLE foo " (the token stream ends with ...WS, EOF).
btw ANTLR emits only one WS token for all consecutive whitespace, so technically the while loop is not necessary. I am keeping it to avoid reader's astonishment.
findepi
force-pushed
the
error-handler
branch
4 times, most recently
from
7e470d1
to
e450457
Compare
And remove one duplicate test case.
When parsing fails in some ambiguous place, the `ErrorHandler` is invoked at last certain position, which might be `expression` or `primaryExpression`. By expanding special rules, we may produce more accurate suggestions.
Window frame bound may be a non-literal expression. This is already covered by existing test case (`io.prestosql.tests.AbstractTestWindowQueries#testWindowFrames`).
No description provided.