Add support for curly brackets in SnowSQL ampersand variables #1901
Conversation
| @@ -105,7 +107,7 @@ | |||
| type="variable", | |||
| ), | |||
| ReferencedVariableNameSegment=RegexParser( | |||
| r"(\$|\&)[A-Z][A-Z0-9_]*", | |||
| r'(?:\$[A-Z][A-Z0-9_]*)|(?:[A-Z0-9_\."]*&{[A-Z0-9_]*}[A-Z0-9_]*)|(?:&[A-Z0-9_\."]*)', | |||
There was a problem hiding this comment.
For some reason (?:[\w\."]*&{[A-Z0-9_]*}[\w]*) does not work, while (?:[A-Z0-9_\."]*&{[A-Z0-9_]*}[A-Z0-9_]*) does. Does anybody know why?
There was a problem hiding this comment.
Those should be identical. Can you define "does not work"? Have you been able to reproduce it on https://regex101.com/?
There was a problem hiding this comment.
Sorry, you are right, "does not work" is rarely helpful.
When I use r'(?:\$[A-Z][A-Z0-9_]*)|(?:[A-Z0-9_\."]*&{[A-Z0-9_]*}[A-Z0-9_]*)|(?:&[A-Z0-9_\."]*)',, then USE WAREHOUSE &WAREHOUSE; parses alright. If I use r'(?:\$[A-Z][A-Z0-9_]*)|(?:[\w\."]*&{[A-Z0-9_]*}[A-Z0-9_]*)|(?:&[\w\."]*)',, the same piece of SQL returns a Found unparsable section error.
I cannot reproduce this on regex101.com - over there both patterns produce the desired match &WAREHOUSE.
There was a problem hiding this comment.
This is a bug - not in your regex - but in the RegexParser that is being used. RegexParser inherits from StringParser, which calls this in its init (which is called in the RegexParser)
self.template = template.upper()
So, \w becomes \W, which matches every character that is not a word character. Basically ^[\w]
…ub.com:chwiese/sqlfluff into snowsql-variables-with-curly-brackets
Codecov Report
@@ Coverage Diff @@
## main #1901 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 147 147
Lines 10219 10219
=========================================
Hits 10219 10219
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
The code changes look good to me. I do have a question: This PR appears to add support for ampersand variables both in the placeholder templater as well as the Snowflake dialect. Do both of them use the same syntax? Is it possible to use both at once? Do we need to add or update documentation so users will understand what's supported and how to use it?
|
@barrywhart Actually this PR removes ampersand support from the Snowflake dialect and moves it to the placeholder templater only. There is still dollar sign variable support in the dialect though. Both can be used concurrently. Dollar sign variables are part of the dialect and can be used with all clients, ampersand variables are specific to the official SnowSQL client. |
|
Got it! Merging now. Great work! |
Brief summary of the change made
Moves support for Snowsql variables from the dialect into the placeholder templater and adds support for curly brackets, as documented here. Since Snowsql variables can hold SQL logic such as optional WHERE clauses, the templater is the better home for them as discussed here on Slack.
Are there any other side effects of this change that we should be aware of?
Yes, anybody how currently has Snowsql variables in their scripts will run into parsing errors. They will need to switch to the placeholder templater and define the placeholders in their Sqlfluff config.
Pull Request checklist
Please confirm you have completed any of the necessary steps below.
Included test cases to demonstrate any code changes, which may be one or more of the following:
.ymlrule test cases intest/fixtures/rules/std_rule_cases..sql/.ymlparser test cases intest/fixtures/dialects(note YML files can be auto generated withpython test/generate_parse_fixture_yml.pyor by runningtoxlocally).test/fixtures/linter/autofix.Added appropriate documentation for the change.
Created GitHub issues for any relevant followup/future enhancements if appropriate.