feat(rules): add W016 not-in-with-subquery rule#26
Conversation
Closes Pawansingh3889#6 NOT IN (SELECT ...) returns zero rows if the subquery contains NULL, which is a common SQL footgun. The rule warns and suggests NOT EXISTS or LEFT JOIN ... IS NULL as alternatives. - Add NotInWithSubquery to sql_guard/rules/warnings.py - Register W016 in ALL_RULES - Add fixture entry to tests/fixtures/warnings.sql - Update rule counts (25 -> 26, 17 -> 18 warnings) - Cover both the trigger case and value-list-ok case in tests
|
Thanks for this, really clean PR. Template is filled out properly, the tests cover both directions, and you spotted the pre-existing structural test failure on main, which saves me time tracking it down separately. One thing I appreciated: the regex You were right to leave the README Key Numbers drift alone. That's mine to fix in the next release commit, not yours to clean up here. Same with the structural test failure, I'll open a separate issue for it. Squash-merging now. Thanks again for taking the time on this one. |
|
Thanks for the merge. |
2 participants
What changed
Adds a new warning rule W016
not-in-with-subquerythat catchesWHERE col NOT IN (SELECT ...). When the subquery returns anyNULL, the predicate evaluates toUNKNOWNfor every outer row and the query silently returns zero results. This is the bug described in issue #6 (the regex and acceptance examples come straight from the issue body).Related issue
Closes #6
If this adds or changes a rule
W016(not used elsewhere; verified viagrep -rn '"W016"' sql_guard/rules)sql_guard/rules/__init__.py(NotInWithSubqueryadded to imports +ALL_RULES, comment updated toW001-W012, W016)tests/fixtures/warnings.sql(the canonicalcustomers NOT IN (SELECT customer_id FROM orders)example from the issue)tests/test_rules.py(test_w016_not_in_with_subqueryruns against the fixture)tmp_path(test_w016_not_in_value_list_okconfirmsNOT IN (1, 2, 3)value lists are not flagged)W016row to the warnings table afterW010). Key Numbers block left as-is since it's already a release behind (says 24 rules / 14 warnings / version 0.4.1 while v0.5.0 is onmainwith 25 rules); fixing that drift seemed out of scope here.Tests
python3 -m pytest tests/test_rules.py -q->32 passed in 0.02s(the two new W016 tests included)ruff check sql_guard/rules/warnings.py sql_guard/rules/__init__.py tests/test_rules.py->All checks passed!The full
pytest tests/run shows one pre-existing failure intests/test_structural.py::TestDeeplyNestedSubquery::test_deep_nesting_detectedthat reproduces onupstream/main(a507f00) before this branch — confirmed by checking out main and running the single test in isolation. Not introduced by this change.Checklist
feat(rules): ...)CHANGELOG.md[Unreleased]entry added under### AddedAnything else reviewers should know
The rule uses
multiline = True+check_statementbecause the issue's footgun pattern naturally spans lines (a real-worldNOT IN (SELECT ...)typically has the subquery on its own line). The chosen regex matches the issue'sr"\bNOT\s+IN\s*\(\s*SELECT\b"exactly, which means it won't trigger on value lists likeNOT IN (1, 2, 3)— that case is asserted bytest_w016_not_in_value_list_ok.This contribution was developed with AI assistance (Codex).