Add Redshift dialect, handle square brackets properly #471
Conversation
We need to detect `[` or `"` for Redshift quotes around indentifier and at the same time exclude treating JSON paths as indentifer
|
Hi @alamb, could you give me some feedback? May the PR be reviewed? |
alamb
changed the title
There was a problem hiding this comment.
Thank you @mskrzypkows for the contribution.
I am sorry for the delay.
I am not super familiar with the Redshift SQL dialect, nor have I used it and I don't have access to such a system. I didn't find any mention of using [ and ] as quote characters in the redshift docs. Here is where I looked: https://docs.aws.amazon.com/redshift/latest/dg/r_names.html
Can you maybe point me at some examples or reference documentation with this syntax?
I think in general this PR is looking good to merge because:
- It is a new Dialect (and thus existing dialects are unaffected)
- It has good test coverate
I left some small style / documentation suggestions
| } | ||
|
|
||
| fn is_identifier_start(&self, ch: char) -> bool { | ||
| PostgreSqlDialect {}.is_identifier_start(ch) |
src/dialect/redshift.rs
Outdated
| /// It's needed to distinguish treating square brackets as quotes from | ||
| /// treating them as json path. If there is identifier then we assume | ||
| /// there is no json path. | ||
| fn is_proper_identifier_inside_quotes(&self, mut _chars: Peekable<Chars<'_>>) -> bool { |
There was a problem hiding this comment.
| fn is_proper_identifier_inside_quotes(&self, mut _chars: Peekable<Chars<'_>>) -> bool { | |
| fn is_proper_identifier_inside_quotes(&self, mut chars: Peekable<Chars<'_>>) -> bool { |
I think typically variables that are not used get prefixed with_ while those that are used do not have the _ prefix.
Pull Request Test Coverage Report for Build 2270327359
💛 - Coveralls |
You're right, it's hard to find some documentation mentioning support for |
…ift_square_brackets
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
…parser-rs into redshift_square_brackets
|
Thanks again @mskrzypkows ! |
|
My pleasure. Glad that it's merged :-) |
|
I hope to create a new sqlparser release later later this week and publish
to crates.io
…On Wed, May 4, 2022 at 8:13 AM Maciej Skrzypkowski ***@***.***> wrote:
My pleasure. Glad that it's merged :-)
—
Reply to this email directly, view it on GitHub
<#471 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADXZMIGMATBJ37QY66OGADVIKH3LANCNFSM5ULSBLLA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
* Redshift square bracket handling We need to detect `[` or `"` for Redshift quotes around indentifier and at the same time exclude treating JSON paths as indentifer * RedshiftSqlDialect documentation update Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Renamed _chars to chars * Fixed warnings * Missing license Co-authored-by: Maciej Skrzypkowski <maciej.skrzypkowski@satoricyber.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
* Redshift square bracket handling We need to detect `[` or `"` for Redshift quotes around indentifier and at the same time exclude treating JSON paths as indentifer * RedshiftSqlDialect documentation update Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org> * Renamed _chars to chars * Fixed warnings * Missing license Co-authored-by: Maciej Skrzypkowski <maciej.skrzypkowski@satoricyber.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
We need to detect
[or"for Redshift quotes around indentifier and at the same time excludetreating JSON paths as indentifer