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 #4309, #4315, #4318. #4319

Merged
merged 4 commits into from
Nov 11, 2024
Merged

Conversation

kaby76
Copy link
Contributor

@kaby76 kaby76 commented Nov 9, 2024

This is a fix for #4309, #4315, and #4318.

This change corrects a lot of issues with the PostgreSQL grammar. The overall issue was that there were still a number of pl/pgsql grammar rules in this grammar even after #4316. As I mentioned, adding the grammar for pl/pgsql directly into the PostgreSQL grammar causes a lot of problems. It is very important to learn from this: you should not merge in the grammar for pl/pgsql into PostgreSQL, unless you are very, very careful. For example, you would have to have every lexer by default work in a different lexer mode. The official parser for PostgreSQL does not try to combine grammars. Why should the Antlr grammar deviate from this?

The changes bring the grammar to be back in line with the official PostgreSQL grammar gram.y. All the tests pass, but more importantly, the ambiguities that I mentioned are now gone. This is because the official Bison grammar is LALR(1), so it shouldn't be ambiguous.

It's not clear if I still have all the pl/pgsql grammar removed from the postgresql grammar. There was one test that uses embedded SQL identifiers, but it shouldn't. So PLSQLVARIABLENAME is still defined and used.

I noticed that the lexer grammar contained numerous "built-in" function names. These were added erroneously. I added the comments from gram.y for the identifier classes into the parser grammar.

@kaby76 kaby76 changed the title [postgresql] Fix for #4318 -- ambiguity in a couple of rules. [postgresql] Fix for #4318, #4309 -- ambiguity in a couple of rules. Nov 9, 2024
@kaby76 kaby76 marked this pull request as draft November 9, 2024 23:44
@kaby76
Copy link
Contributor Author

kaby76 commented Nov 9, 2024

(Need to resolve conflicts once #4316 is merged.)

@kaby76 kaby76 changed the title [postgresql] Fix for #4318, #4309 -- ambiguity in a couple of rules. [postgresql] Fix for #4309, #4315, #4318. Nov 10, 2024
@kaby76 kaby76 marked this pull request as ready for review November 10, 2024 22:56
@teverett
Copy link
Member

@kaby76 thanks!

@teverett teverett merged commit 0120854 into antlr:master Nov 11, 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