Handle recovery for statements with invalid leading tokens#288
Open
florinutz wants to merge 1 commit into
Open
Handle recovery for statements with invalid leading tokens#288florinutz wants to merge 1 commit into
florinutz wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumping the pinned grammar to CrateDB 6.3.2 turned the test suites red on May 23:
IndexErrorin Python,TypeErrorin JavaScript.Root cause
6.3.2 made the leading
statementoptional in the grammar (statements: statement (SEMICOLON statement)* ...→statement?). As a result, a statement whose first token is invalid (e.g.SELCT 2) no longer produces aStatementContext— the parse collapses to error nodes — sosqlparse()returned an empty list and callers indexing[0]blew up. Empty input / a bare;are now valid, which is the intended upstream change; the parser wrapper just didn't account for it.Fix
When the parse yields no statements but an error was collected, rebuild from it: emit one
Statementcarrying the error, then recover the remainder after the first top-level;(recursing, since the tail can collapse again). Applied identically to the Python and JavaScript targets.Also in this PR
expected_messageassertions to 6.3.2's token set (it now legitimately lists<EOF>and';').for..in, JS) instead of the query strings.Known limitation
A bad statement in a non-leading position still derails the statements after it — the older, still-open #28. It's captured here as a strict-
xfail(Python) /test.fails(JS) tripwire that will flip the day #28 is fixed.Checklist
21 passed, 1 xfailed; JS23 passed, 1 expected fail)CHANGES.mdCloses #284