feat(rules): add W011 union-without-all warning rule#12
Merged
Pawansingh3889 merged 1 commit intoPawansingh3889:mainfrom Apr 19, 2026
Merged
Conversation
Implements the `W011 union-without-all` rule requested in Pawansingh3889#1. `UNION` forces a sort-and-dedupe pass; `UNION ALL` concatenates. The rule warns when `UNION` is used without `ALL`, letting the author actively decide whether duplicate-removal is actually wanted. Mirrors the single-statement multiline shape of the existing `SubqueryCouldBeJoin` (W005) rule. Regex `\bUNION\b(?!\s+ALL\b)` matches bare `UNION` but not `UNION ALL`. Added to `ALL_RULES`; registry-count assertions updated from 19 -> 20 and W-series warnings from 13 -> 14. Tests: - `tests/fixtures/warnings.sql` gets a triggering case (`UNION` between two `orders_*` selects) - `test_w011_union_without_all` checks that a W011 finding is reported - `test_w011_passes_on_union_all` constructs the rule directly and verifies `UNION ALL` is NOT flagged Fixes Pawansingh3889#1
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for your first PR on sql-sop! Quick checks:
pytest -qandruff check .pass locally- Every new rule has both a "fires on bad SQL" test AND a
"does NOT fire on safe SQL" test - Rule IDs (
E001,W001, …) are public API — don't renumber
or rename existing ones without the deprecation process in
GOVERNANCE.md CHANGELOG.md[Unreleased]entry for any user-facing change
First-PR-wins on the linked issue applies for 7 days. The
maintainer will review within 14 days of your last response.
Pawansingh3889
changed the title
Pawansingh3889
approved these changes
Apr 19, 2026
Owner
There was a problem hiding this comment.
🎉 Nice first PR, thanks.
Matches the W005 template, both tests in place (the UNION ALL regression is a good call), 80 passing locally ✅
Renamed the title so the conventional-commits check passes - that's what validate was failing on. The test_duration_tracked thing you flagged is a known flaky test, not from this pr.
Merging, closes #1. #2-10 are in the same shape if you fancy another 🙌
Pawansingh3889
added a commit
that referenced
this pull request
Apr 27, 2026
Trevin Chow shipped two rules pre-v0.6 that were missed when the Contributors section first landed: - W011 union-without-all (PR #12) - P005 sqlalchemy-text-fstring (PR #25), one of the five Python rules Reordered the table chronologically by first contribution so the thank-you reads in joining order: tmchow, mvanhorn, Prabhu-1409.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
W011 union-without-allper #1. The rule warns whenUNIONis used withoutALL, since bareUNIONforces a sort-and-dedupe pass that is often unnecessary when the two sides are disjoint (different date-range tables, different customer IDs, etc.).Why this matters
The issue body flags a real cost:
UNIONforces a sort-and-dedupe pass whileUNION ALLconcatenates. On large result sets the difference is often an order of magnitude.W011is an advisory warning so the author has to actively justify the dedupe cost.Mirrors the single-statement multiline shape of the existing
SubqueryCouldBeJoin(W005) rule that the issue cited as the template. Regex�UNION�(?!\s+ALL�)matches bareUNIONbut notUNION ALL.Changes
sql_guard/rules/warnings.py: newUnionWithoutAllclass (W011). Copies the shape ofSubqueryCouldBeJoin/OrderByWithoutLimit- multiline statement check,Rule._compile(...)pattern, standardFindingreturn.sql_guard/rules/__init__.py: importUnionWithoutAll, register it inALL_RULESafterCommentedOutCode, and bump the# Warnings (W001-W010)section comment toW001-W011.tests/fixtures/warnings.sql: add a triggering case (UNIONbetween twoorders_*selects).tests/test_rules.py:test_all_rules_loaded: 19 -> 20test_13_warnings->test_14_warnings, 13 -> 14 (W-series goes 10 -> 11, plus the 3 structural)test_w011_union_without_all: asserts a W011 finding fires on the fixturetest_w011_passes_on_union_all: constructsUnionWithoutAll()directly and assertsUNION ALLis NOT flagged (regression test for the lookahead)Testing
The pre-existing
TestChecker.test_duration_trackedfailure (assert 0.0 > 0) is onmaintoo; fast machines complete the check faster than the resolution oftime.perf_countercaptures. Not touched here.Fixes #1
This contribution was developed with AI assistance (Claude Code).