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
Fix subquery bug in L026 #1948
Fix subquery bug in L026 #1948
Conversation
…ntially impacted rules
… into L026_subquery_bug
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.
LGTM if the tests pass.
@pwildenhain too late to get into 0.8.2? Will let you merge if you're happy to include. If not will merge after.
Codecov Report
@@ Coverage Diff @@
## main #1948 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 147 147
Lines 10219 10219
=========================================
Hits 10219 10219
Continue to review full report at Codecov.
|
Does this fix #1462 as well out of interest? Happy to merge as is but thought worth asking if it does, or if it’s related and could be made to, before I merge. |
Will take a look into it 👀 |
@tunetheweb I've checked and no it doesn't seem to impact that issue so I think we can consider it out of scope for this PR. Let's merge this one in and then happy to investigate the other issue further seperately 😄 |
Brief summary of the change made
This PR fixes bug in L026 in which table references within subqueries were incorrectly being identified as not being found. Fixes #1939.
On investigating I found the cause was the
references
returned fromget_select_statement_info
incorrectly included references from subqueries. There was already some logic inget_select_statement_info
to handle this but was in the wrong place in the function. I have moved this logic to the end of the function and added extra unit tests to all rules which utiliseget_select_statement_info
to provide additional validation.Are there any other side effects of this change that we should be aware of?
Not that I'm aware of, I've added additional unit tests to be sure that other rules aren't impacted.
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:
.yml
rule test cases intest/fixtures/rules/std_rule_cases
..sql
/.yml
parser test cases intest/fixtures/dialects
(note YML files can be auto generated withpython test/generate_parse_fixture_yml.py
or by runningtox
locally).test/fixtures/linter/autofix
.Added appropriate documentation for the change.
Created GitHub issues for any relevant followup/future enhancements if appropriate.