fix(trino): catch up with SQLFluff and stop panicking on common statements - #3046
Open
dkxmercury wants to merge 1 commit into
Open
fix(trino): catch up with SQLFluff and stop panicking on common statements#3046dkxmercury wants to merge 1 commit into
dkxmercury wants to merge 1 commit into
Conversation
…ments
Trino is built from the ANSI dialect and then clears its keyword sets, but
the inherited ANSI grammar still referenced keywords Trino no longer defines.
Parsing hit the None arm of Dialect::ref and panicked on everyday SQL:
create table -> Grammar refers to 'TRANSIENT'
insert into t values (1) -> Grammar refers to 'OVERWRITE'
alter table t -> Grammar refers to 'SEQUENCE'
Ports the Trino changes from the SQLFluff commits listed in the issue:
c6b39aae TemporaryTransientGrammar becomes Nothing
d36f2f06 explicit Trino StatementSegment, CREATE TABLE, column and
transaction grammars, and the FUNCTION keyword
74aaf399 Trino INSERT without OVERWRITE, plus COMMIT, ROLLBACK and
SET SESSION
07db73d1 Trino ALTER TABLE without unsupported inherited options
Listing the supported statements explicitly is what keeps unsupported ANSI
branches such as sequences out of the dialect, so a missing keyword can no
longer be reached. Also adds a lexer matcher for `=>` so ALTER TABLE EXECUTE
arguments lex as one token.
Fixtures for CREATE TABLE, ALTER TABLE, INSERT, COMMIT, ROLLBACK,
START TRANSACTION and SET SESSION come from the same upstream commits.
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.
Fixes #3017.
Summary
Implements the four SQLFluff commits you listed, so the Trino dialect stops panicking on everyday statements and picks up the upstream grammar it had missed.
Before this, parsing any of these panicked instead of producing a tree or a clean error:
What was ported
c6b39aaeTemporaryTransientGrammarbecomesNothing, which removes the inheritedTRANSIENTreference.PrimaryKeyGrammar,ForeignKeyGrammar,UniqueKeyGrammarandTemporaryGrammarare silenced the same way, matching upstream.d36f2f06an explicit TrinoStatementSegmentlisting only the statements Trino supports, plus TrinoCREATE TABLE,ColumnDefinitionSegmentandTransactionStatementSegment. Also adds theFUNCTIONkeyword, which that commit added to the Trino keyword list.74aaf399TrinoINSERTwithoutOVERWRITE, andCOMMIT,ROLLBACKandSET SESSION.07db73d1TrinoALTER TABLE, covering RENAME, ADD/DROP/RENAME/ALTER COLUMN, SET AUTHORIZATION, SET PROPERTIES and EXECUTE, without unsupported inherited options.Listing the supported statements explicitly is the part that actually closes off the panics, since unsupported ANSI branches such as sequences are no longer reachable and cannot resolve a keyword Trino does not define.
One addition beyond a direct port:
ALTER TABLE ... EXECUTE f(x => 1)needs=>to lex as a single token, so the dialect now inserts afat_right_arrowlexer matcher alongside the existingright_arrow, mirroringStringLexer("fat_right_arrow", "=>", ...)upstream. Without it the argument list is unparsable.Testing
create table,create table t (a integer),insert into t values (1),insert into t (a) select 1,alter table t rename to u,alter table t add column c integer,alter table t execute f(x => 1),commit,rollback,start transaction isolation level serializableandset session foo = 1all parse.cargo test -p sqruff-lib-dialects --test dialectsrun is clean, and I ran it twice before and after the change to be sure the result was stable.cargo test -p sqruff-lib-coreis green,cargo fmt --checkandcargo clippy -p sqruff-lib-dialects --tests -- -Dwarningsare clean.One local note in case it helps anyone else on Windows: the fixture tests are sensitive to line endings, and with
core.autocrlf=truea set of unrelated fixtures with multi line string bodies fails and the failing set varies between runs. With an LF checkout the suite is stable and green, which is what I used for the comparison above.