fix: is_select check for lowercase select with "WITH" clauses #22370
Conversation
Checking for token.normalized instead of token.value in is_select check, avoiding issues with lowercase select statements
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️
We hope to see you in our Slack community too!
Codecov Report
@@ Coverage Diff @@
## master #22370 +/- ##
=======================================
Coverage 66.87% 66.87%
=======================================
Files 1847 1847
Lines 70561 70561
Branches 7739 7739
=======================================
Hits 47186 47186
Misses 21374 21374
Partials 2001 2001
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
This is great, @fcomuniz!
Would you mind adding a unit test to tests/unit_tests/sql_parse_tests.py?
|
I added a lowercase select test to make sure it works correctly |
|
@fcomuniz there appears to be a linting issue in your test: |
|
@villebro, how can i check the linting, is there a tutorial anywhere for running this? Thanks. |
|
unit test is added, good for merge? @betodealmeida |
mistercrunch
added
🏷️ bot
Checking for token.normalized instead of token.value in is_select check, avoiding issues with lowercase select statements
SUMMARY
is_select check in a ParsedQuery object fails to give true response when has multiple WITH clauses, such that sqlparse gives an UNKNOWN type for the query when the statement uses a lowercase "select" clause. This shouldn't happen as lowercase select statements are still select statements
TESTING INSTRUCTIONS
test updating the metadata for a virtual dataset with "WITH" clause at the beginning with lowercase select statements
ADDITIONAL INFORMATION