From 2262ea8d138ef45487b87e2e8cd9f89352b4d660 Mon Sep 17 00:00:00 2001 From: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com> Date: Sat, 9 May 2026 17:07:45 +0100 Subject: [PATCH 1/6] Add W024 select-distinct-suspicious rule Fires when a statement contains both top-level SELECT DISTINCT and any JOIN keyword. The pattern is a frequent band-aid for a missing join condition or a missing GROUP BY: developers reach for DISTINCT to swallow row duplication caused by an over-wide JOIN cardinality rather than fixing the join. The regex anchors on SELECT directly preceding DISTINCT so aggregate- DISTINCT forms like COUNT(DISTINCT col) and SUM(DISTINCT amount) are not flagged. String literals and comments are stripped first via strip_strings_and_comments so a mention of SELECT DISTINCT inside a literal or a comment cannot trigger the rule. multiline = True so the DISTINCT and JOIN keywords can sit on different lines of the same statement. Resolves #40. --- sql_guard/rules/warnings.py | 52 ++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/sql_guard/rules/warnings.py b/sql_guard/rules/warnings.py index 88608e8..132b87f 100644 --- a/sql_guard/rules/warnings.py +++ b/sql_guard/rules/warnings.py @@ -4,7 +4,7 @@ import re -from sql_guard.rules.base import Finding, Rule +from sql_guard.rules.base import Finding, Rule, strip_strings_and_comments class SelectStar(Rule): @@ -510,6 +510,56 @@ def check_line(self, line: str, line_number: int, file: str) -> Finding | None: return None +class SelectDistinctSuspicious(Rule): + """W024: ``SELECT DISTINCT`` paired with ``JOIN`` is often a band-aid. + + Developers reach for ``SELECT DISTINCT`` to deduplicate rows when a + JOIN cardinality is wider than they expected, often because a join + condition is missing or because what they actually want is a + ``GROUP BY``. The result reads more rows than necessary and hides + the underlying join-condition bug. + + Fires when a statement contains both top-level ``SELECT DISTINCT`` + and any ``JOIN`` keyword. Standalone ``SELECT DISTINCT col FROM t`` + (no join) is fine -- the smell is the cross-join blow-up pattern, + not legitimate single-table uniqueness. + + ``COUNT(DISTINCT col)`` and other aggregate-DISTINCT forms are not + flagged: the regex anchors on ``SELECT`` directly preceding + ``DISTINCT``, so ``SELECT COUNT(DISTINCT x) FROM t JOIN u`` does + not match. + """ + + id = "W024" + name = "select-distinct-suspicious" + severity = "warning" + description = "SELECT DISTINCT combined with JOIN often masks a missing join condition" + multiline = True + + _select_distinct = Rule._compile(r"\bSELECT\s+DISTINCT\b") + _join = Rule._compile(r"\bJOIN\b") + + def check_statement(self, statement: str, start_line: int, file: str) -> Finding | None: + # Strip strings and comments so DISTINCT or JOIN inside literal + # text or comments cannot trigger a false positive. + stripped = strip_strings_and_comments(statement) + if self._select_distinct.search(stripped) and self._join.search(stripped): + return Finding( + rule_id=self.id, + severity=self.severity, + file=file, + line=start_line, + message=( + "SELECT DISTINCT with JOIN often masks a missing join condition or grouping" + ), + suggestion=( + "Verify the JOIN condition is correct and DISTINCT is genuinely needed; " + "consider GROUP BY if you are hiding row duplication" + ), + ) + return None + + class CountDistinctUnbounded(Rule): """W019: ``COUNT(DISTINCT col)`` on an unfiltered table. From f52648155e4cc87fec41794540efff6c7579f0ac Mon Sep 17 00:00:00 2001 From: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com> Date: Sat, 9 May 2026 17:07:53 +0100 Subject: [PATCH 2/6] Register W024 in rules registry; bump test counts to 42 / 31 warnings --- sql_guard/rules/__init__.py | 2 ++ tests/test_rules.py | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/sql_guard/rules/__init__.py b/sql_guard/rules/__init__.py index f546170..a7bdc6a 100644 --- a/sql_guard/rules/__init__.py +++ b/sql_guard/rules/__init__.py @@ -33,6 +33,7 @@ OrAcrossColumns, OrderByWithoutLimit, ScalarUdfInWhere, + SelectDistinctSuspicious, SelectStar, SubqueryCouldBeJoin, TruncateTable, @@ -106,6 +107,7 @@ LeadingWildcardLike(), OrAcrossColumns(), CountDistinctUnbounded(), + SelectDistinctSuspicious(), TruncateTable(), HavingWithoutGroupBy(), CrossJoinExplicit(), diff --git a/tests/test_rules.py b/tests/test_rules.py index 1e9397c..824a02c 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -18,19 +18,19 @@ class TestRuleRegistry: def test_all_rules_loaded(self) -> None: - assert len(ALL_RULES) == 41 + assert len(ALL_RULES) == 42 def test_11_errors(self) -> None: # 9 E-series + 2 T-series (T002 xp-cmdshell, T004 deprecated-outer-join). errors = [r for r in ALL_RULES if r.severity == "error"] assert len(errors) == 11 - def test_30_warnings(self) -> None: - # 23 W-series + 3 S-series + 4 T-series (T001 with-nolock, + def test_31_warnings(self) -> None: + # 24 W-series + 3 S-series + 4 T-series (T001 with-nolock, # T003 cursor-declaration, T005 create-index-without-online, # T006 select-into-without-typed-fields). warnings = [r for r in ALL_RULES if r.severity == "warning"] - assert len(warnings) == 30 + assert len(warnings) == 31 def test_unique_ids(self) -> None: ids = [r.id for r in ALL_RULES] From e42d27c6a128b56bc5f3cf7e4624748a8c213965 Mon Sep 17 00:00:00 2001 From: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com> Date: Sat, 9 May 2026 17:08:04 +0100 Subject: [PATCH 3/6] Add W024 select-distinct-suspicious tests Covers: INNER JOIN, LEFT JOIN, multi-line, case-insensitive, single-table DISTINCT (passes), COUNT(DISTINCT) with JOIN (passes), SUM(DISTINCT) with JOIN (passes), JOIN without DISTINCT (passes), DISTINCT inside a string literal (passes), DISTINCT inside a -- comment (passes), and a message-content assertion. --- tests/test_new_rules.py | 83 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/test_new_rules.py b/tests/test_new_rules.py index 182c250..9f62ea2 100644 --- a/tests/test_new_rules.py +++ b/tests/test_new_rules.py @@ -10,6 +10,7 @@ LeadingWildcardLike, OrAcrossColumns, ScalarUdfInWhere, + SelectDistinctSuspicious, TruncateTable, ) @@ -387,3 +388,85 @@ def test_w022_does_not_flag_cross_join_inside_trailing_comment(): ) is None ) + + +# W024 select-distinct-suspicious --------------------------------------------- + + +def test_w024_flags_distinct_with_inner_join(): + rule = SelectDistinctSuspicious() + finding = _stmt( + rule, + "SELECT DISTINCT c.id, c.name FROM customers c JOIN orders o ON c.id = o.customer_id;", + ) + assert finding is not None + assert finding.rule_id == "W024" + assert finding.severity == "warning" + + +def test_w024_flags_distinct_with_left_join(): + rule = SelectDistinctSuspicious() + sql = "SELECT DISTINCT a.id FROM a LEFT JOIN b ON a.id = b.a_id;" + assert _stmt(rule, sql) is not None + + +def test_w024_flags_distinct_with_join_multiline(): + rule = SelectDistinctSuspicious() + sql = "SELECT DISTINCT\n c.id, c.name\nFROM customers c\nJOIN orders o ON c.id = o.customer_id;" + assert _stmt(rule, sql) is not None + + +def test_w024_case_insensitive(): + rule = SelectDistinctSuspicious() + assert _stmt(rule, "select distinct a from x join y on x.id = y.id;") is not None + + +def test_w024_does_not_flag_distinct_alone(): + # Single-table DISTINCT is fine -- no JOIN cardinality blow-up to mask. + rule = SelectDistinctSuspicious() + assert _stmt(rule, "SELECT DISTINCT country FROM customers;") is None + + +def test_w024_does_not_flag_count_distinct_with_join(): + # Aggregate-DISTINCT is a different pattern; the regex anchors on + # "SELECT DISTINCT" directly, not "COUNT(DISTINCT ...)". + rule = SelectDistinctSuspicious() + sql = "SELECT COUNT(DISTINCT c.id) FROM customers c JOIN orders o ON c.id = o.customer_id;" + assert _stmt(rule, sql) is None + + +def test_w024_does_not_flag_sum_distinct_with_join(): + rule = SelectDistinctSuspicious() + sql = "SELECT SUM(DISTINCT amount) FROM payments p JOIN customers c ON p.cust_id = c.id;" + assert _stmt(rule, sql) is None + + +def test_w024_does_not_flag_join_without_distinct(): + rule = SelectDistinctSuspicious() + assert ( + _stmt(rule, "SELECT a.id FROM a JOIN b ON a.id = b.a_id;") is None + ) + + +def test_w024_does_not_flag_distinct_inside_string_literal(): + rule = SelectDistinctSuspicious() + sql = "INSERT INTO log(msg) SELECT 'SELECT DISTINCT x JOIN y' FROM t JOIN u ON t.id = u.id;" + # Outer SELECT does not have DISTINCT; the literal mentions it. + assert _stmt(rule, sql) is None + + +def test_w024_does_not_flag_distinct_inside_comment(): + rule = SelectDistinctSuspicious() + sql = "-- SELECT DISTINCT x FROM y JOIN z\nSELECT id FROM t;" + assert _stmt(rule, sql) is None + + +def test_w024_message_mentions_join_or_grouping(): + rule = SelectDistinctSuspicious() + finding = _stmt( + rule, + "SELECT DISTINCT a FROM x JOIN y ON x.id = y.id;", + ) + assert finding is not None + msg = finding.message.lower() + assert "join" in msg or "grouping" in msg From 889144bfc40dc59eb7d628ad57912e781117a2a9 Mon Sep 17 00:00:00 2001 From: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com> Date: Sat, 9 May 2026 17:08:13 +0100 Subject: [PATCH 4/6] Document W024 in CHANGELOG under [Unreleased] Added --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39aaed0..bec65de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,17 @@ a deprecation window (see `GOVERNANCE.md` ยง Scope discipline). the explicit-column variant (`SELECT col1, col2 INTO target FROM source`) is allowed through here and covered by the contracts pack at C001/C003 if a column type drifts. Resolves #43. +- **W024 `select-distinct-suspicious`** (warning) - fires when a + statement contains both top-level `SELECT DISTINCT` and any `JOIN` + keyword. The pattern is a common band-aid for a missing join + condition or a missing `GROUP BY`: developers reach for `DISTINCT` + to swallow row duplication caused by an over-wide JOIN cardinality + rather than fixing the join. Standalone `SELECT DISTINCT col FROM t` + (no join) is left alone, and aggregate-DISTINCT forms like + `COUNT(DISTINCT col)` are not flagged because the regex anchors on + `SELECT` directly preceding `DISTINCT`. String literals and comments + are stripped first so a mention of "SELECT DISTINCT" inside a string + or comment cannot fire the rule. Resolves #40. ### Fixed From 9f158abdfc2358a42b796682ec54c66c81bdb988 Mon Sep 17 00:00:00 2001 From: Pawan Singh Kapkoti <42340841+Pawansingh3889@users.noreply.github.com> Date: Sat, 9 May 2026 17:08:13 +0100 Subject: [PATCH 5/6] Add W024 row to README rule table --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 6ff7848..54ea4e7 100644 --- a/README.md +++ b/README.md @@ -238,6 +238,7 @@ sql-sop list-rules # show every registered rule | W020 | `truncate-table` | `TRUNCATE TABLE staging;` -- bypasses triggers, resets identity | | W022 | `cross-join-explicit` | `FROM products CROSS JOIN regions` -- Cartesian product, confirm intent | | W023 | `scalar-udf-in-where` | `WHERE dbo.fn_X(col) = 1` -- row-by-row predicate evaluation | +| W024 | `select-distinct-suspicious` | `SELECT DISTINCT a, b FROM x JOIN y ON ...` -- DISTINCT often masks a missing join condition or GROUP BY | ### Structural (v0.3.0+, sqlparse-based) From 35a8e392e0481db3f9aa45ea4b87ad65723c35b6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 9 May 2026 16:30:43 +0000 Subject: [PATCH 6/6] style: pre-commit auto-fixes [pre-commit.ci] auto-applied fixes from configured hooks --- tests/test_new_rules.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_new_rules.py b/tests/test_new_rules.py index 9f62ea2..b5d31ce 100644 --- a/tests/test_new_rules.py +++ b/tests/test_new_rules.py @@ -412,7 +412,9 @@ def test_w024_flags_distinct_with_left_join(): def test_w024_flags_distinct_with_join_multiline(): rule = SelectDistinctSuspicious() - sql = "SELECT DISTINCT\n c.id, c.name\nFROM customers c\nJOIN orders o ON c.id = o.customer_id;" + sql = ( + "SELECT DISTINCT\n c.id, c.name\nFROM customers c\nJOIN orders o ON c.id = o.customer_id;" + ) assert _stmt(rule, sql) is not None @@ -443,9 +445,7 @@ def test_w024_does_not_flag_sum_distinct_with_join(): def test_w024_does_not_flag_join_without_distinct(): rule = SelectDistinctSuspicious() - assert ( - _stmt(rule, "SELECT a.id FROM a JOIN b ON a.id = b.a_id;") is None - ) + assert _stmt(rule, "SELECT a.id FROM a JOIN b ON a.id = b.a_id;") is None def test_w024_does_not_flag_distinct_inside_string_literal():