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

sql:tsql - grammar converted to case-insensitive #2417

Closed

Conversation

OleksiiKovalov
Copy link
Contributor

@OleksiiKovalov OleksiiKovalov commented Dec 7, 2021

What does this PR do?
Makes grammar case insensitive
minor fixes to avoid c# warnings (EMPTY literal renamed to EMPTY_)

Why?
By standard SQL is case-insensitive when parsing keywords.
shame on me but I was unable to find SQL Server documentation that directly confirms that.

How is it checked?
using standard post-commit checks/ci

fixed collision of EMPTY literal with antlr EMPTY property;
added TRY_PARSE() function recognition
added VERSION as a "Keyword as identifier"
@KvanTTT KvanTTT added case-insensitive Case insensitive lexing, https://github.com/antlr/antlr4/blob/master/doc/case-insensitive-lexing.md tsql labels Dec 7, 2021
@OleksiiKovalov OleksiiKovalov marked this pull request as ready for review December 7, 2021 13:36
@KvanTTT
Copy link
Member

KvanTTT commented Dec 7, 2021

Hi! Thank you for the request but I postpone it for now. I don't like transformation to such fragments in all our case insensitive grammars (T-Sql, MySql, PlSql, Pascal ant others). I would like to ask @parrt to consider again introducing the new option options { caseInsensitive=true } that will get us rid of big amount of useless and ugly changes. I have such improvements but they need to be updated. There are a lot of problems for users because of absence of such an option: https://github.com/antlr/grammars-v4/issues?q=label%3Acase-insensitive

@OleksiiKovalov
Copy link
Contributor Author

hi @KvanTTT ,
good, let it be postponed till decision.
I'll proceed with other changes in another branch (eg - making grammar "wider" 'cause actual parser does not follow documentation and current grammar fails on some real-world scripts)

@kaby76
Copy link
Contributor

kaby76 commented Dec 8, 2021

@OleksiiKovalov @KvanTTT I should note that I just added the transform in Trash called "trull" ("upper/lowercase literal") that can rewrite the lexer rules with case insensitivity using lexer character sets. One would have to change the pom.xml to remove the case folding specification to have a complete, consistent rewrite, but it's something I will probably add to the transform since the pom.xml is part of the grammar. The transform doesn't introduce the fragments for individual letters, instead uses the lexer set directly, eg, 'abc' => [aA] [bB] [cC], nor the inverse. Again, something I'll eventually get around to implementing. So, making a private copy with the transform for yourself is completely automatable, quick, and easy.

@KvanTTT
Copy link
Member

KvanTTT commented Dec 25, 2021

Also, waiting for the next version of ANTLR with caseInsensitive option. But anyway thank you for the contribution. Please create separated PR with the rest issues.

@KvanTTT KvanTTT closed this Dec 25, 2021
@parrt
Copy link
Member

parrt commented Dec 27, 2021

we have merged the case insensitive option by @KvanTTT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
case-insensitive Case insensitive lexing, https://github.com/antlr/antlr4/blob/master/doc/case-insensitive-lexing.md tsql
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants