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.

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

[postgresql] Fix for #4302 -- ambiguity in stmtmulti #4303

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

kaby76
Copy link
Contributor

@kaby76 kaby76 commented Nov 2, 2024

This is a fix for #4302.

Consider the input VACUUM ANALYZE brin_test;. This can be parsed with the current grammar in two ways:

  • stmtmulti => stmt SEMI ==> VACUUM ANALYZE brin_test ;.
  • stmtmulti => stmt stmt SEMI ==>VACUUM ANALYZE brin_test ;. It is very likely this is meant to be one statement.

The problem is how stmt are separated. In the official gram.y grammar, semicolon has to separate the two. Otherwise it should be considered as one stmt.

This change modifies the rules for meta-commands, which are not part of SQL, and the rules in the parser associated with SQL command separation. The changes in #2365 modified these rules in an incorrect way, introducing ambiguity in the grammar.

  1. The lexer rules for meta-commands are modified to return a SEMI, because that is essentially how they function. The meta-command \' is special. it must return a SEMI because that is how it is defined in the documentation at https://www.postgresql.org/docs/current/app-psql.html#APP-PSQL-META-COMMAND-SEMICOLON. They don't tell you there that meta-commands work this way also, but they do. As before, there is no attempt to "parse" the meta-commands themselves.
  2. SQL commands must be separated by a SEMI, as per the official Bison grammar for PostgreSQL.

The ambiguity for opt_analyze is eliminated. dotnet trperf -i 'VACUUM ANALYZE brin_test;' -c ard | grep -v '^0' yields no output (as trperf outputs in column 1 the number of ambiguity detected, and grep removes all lines that begin with '0').

I should note that the parse time for examples/*.sql is not faster (37s after vs 44s before).

Also, I really do not understand why there are error listeners defined for this grammar. These have nothing to do with the grammar and must be removed.

@kaby76 kaby76 changed the title [postgresql] Fix for #4302 [postgresql] Fix for #4302 -- ambiguity in stmtmulti Nov 4, 2024
@teverett
Copy link
Member

teverett commented Nov 4, 2024

@kaby76 thanks!

@teverett teverett merged commit eace15f into antlr:master Nov 4, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants