-
Notifications
You must be signed in to change notification settings - Fork 583
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
Support for empty array literals (ARRAY[]
)
#532
Support for empty array literals (ARRAY[]
)
#532
Conversation
Hey @bitemyapp , thanks for the PR!
Yes please, that sounds like the best way forward. |
Pull Request Test Coverage Report for Build 2625673440
💛 - Coveralls |
Okie dokie artichokie. I'll push it tomorrow. |
@Dandandan oh, actually, where should the test specifically for the |
2053cd4
to
6f09301
Compare
Squashed the commits, tests pass locally. |
Amended the MR title to fit the reality better. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
@bitemyapp could you fix the code style issues? Otherwise this is good to go! |
ARRAY[]
)
6f09301
to
ff82113
Compare
@Dandandan it should be fixed now. FWIW that license template header option seems to be gone in newer versions of |
Thank you @bitemyapp ! |
Co-authored-by: Chris Allen <[email protected]>
We had SQL queries that contained fragments that looked like this:
We needed to coalesce the values down to empty arrays. This is the recommended syntax on the AWS Athena docs: https://docs.aws.amazon.com/athena/latest/ug/creating-arrays.html
Oddly, the parser assumed all
ARRAY
keyword literal values would have a comma separated list of values inside of them. This initially threw me off as I assumed it would be a lexing problem. That's one reason there's an AthenaSqlDialect in here. The other reason is that I wasn't sure if I should bolt theARRAY[]
test onto an existing dialect or carve out an Athena dialect. The Athena SQL dialect tests are a hodgepodge of Hive tests with a single array literal test from PostgreSQL adapted to this specific missing feature.If you want to migrate the test into an existing dialect and nuke the Athena dialect, that's fine by me. Athena SQL is basically Presto+Hive I think, so it's whatever you want to do. This problem is fixed for all SQL dialects regardless because it's in the parser.
Cheers and thanks for the parser, this thing has saved me countless hours.