Skip to content

feat(rules): add W021 having-without-group-by#39

Merged
Pawansingh3889 merged 2 commits intoPawansingh3889:mainfrom
mvanhorn:osc/3-add-w021-having-without-group-by
May 3, 2026
Merged

feat(rules): add W021 having-without-group-by#39
Pawansingh3889 merged 2 commits intoPawansingh3889:mainfrom
mvanhorn:osc/3-add-w021-having-without-group-by

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

@mvanhorn mvanhorn commented May 3, 2026

Summary

Adds W021, a warning rule that flags HAVING without a preceding GROUP BY in the same query block. The SQL engine treats this as a single implicit group and produces results that surprise the author -- almost always a typo for WHERE.

What changed

  • sql_guard/rules/warnings.py -- new HavingWithoutGroupBy class. Mirrors W006 (OrderByWithoutLimit) for structure (multiline rule, two compiled regexes, check_statement returns Finding | None). Strips line and block comments before matching, and only accepts GROUP BY matches at parenthesis depth 0 so a GROUP BY inside a subquery does not satisfy a HAVING in the outer query.
  • sql_guard/rules/__init__.py -- imports HavingWithoutGroupBy, slots it into ALL_RULES between TruncateTable (W020) and CrossJoinExplicit (W022) to fill the W021 gap.
  • tests/fixtures/warnings.sql -- adds the canonical SELECT total FROM orders HAVING total > 1000; example.
  • tests/test_rules.py -- adds test_w021_having_without_group_by (fixture-driven), plus two unit tests covering the comment and subquery cases. Updates rule-count assertions: test_all_rules_loaded 38 → 39, test_28_warnings renamed to test_29_warnings and bumped 28 → 29 (W-series goes from 22 to 23).

Verification

python -m pytest tests/test_rules.py -v
42 passed in 0.03s

Known limitation

A SELECT ... FROM (SELECT ... GROUP BY ... HAVING ...) x -- valid SQL where a grouped subquery uses its own HAVING -- can produce a W021 false positive. The rule finds the first HAVING anywhere in the statement and looks for a depth-0 GROUP BY before it; the nested HAVING is depth >0 and its GROUP BY also depth >0, so the depth-0 search misses it. Fixing this cleanly requires depth-aware HAVING/GROUP BY pairing, which is more invasive than W006's regex style and worth a follow-up if it shows up in real fixtures. Filing as-is because the simple form (the one the issue calls out) is correctly detected.

Closes #3

This contribution was developed with AI assistance.

Detects HAVING clauses without a preceding GROUP BY in the same
statement. The SQL engine treats HAVING-without-GROUP BY as a single
implicit group and produces results that surprise the author -- almost
always a typo for WHERE.

Mirrors the W006 OrderByWithoutLimit pattern: multiline rule, two
compiled regexes (HAVING / GROUP BY), check_statement returns a Finding
when HAVING appears without GROUP BY (or with GROUP BY appearing after
HAVING). Warning severity.

Closes Pawansingh3889#3
@mvanhorn mvanhorn requested a review from Pawansingh3889 as a code owner May 3, 2026 07:38
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 3, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
sql_guard/rules/warnings.py 96.42% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Pawansingh3889 Pawansingh3889 force-pushed the osc/3-add-w021-having-without-group-by branch from 289fd7c to 6dc08bb Compare May 3, 2026 08:19
@Pawansingh3889
Copy link
Copy Markdown
Owner

Thanks Matt. Fourth contributed rule and the quality bar keeps going up. The depth-aware GROUP BY detection is the right call, and the upfront acknowledgement of the nested-subquery edge case is the kind of disclosure a maintainer always wants to see.

Squashing in.

@Pawansingh3889 Pawansingh3889 merged commit db78ff9 into Pawansingh3889:main May 3, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rule: W021 having-without-group-by - warn on HAVING without GROUP BY

3 participants