Skip to content

Commit ec34b41

Browse files
authored
feat(rules): report queries that cannot do what they appear to (#15)
* feat(rules): report queries that cannot do what they appear to Add eight report-only Rules on the parsed statements and tokens: duplicate CTE names (SQC046), CASE WHEN NULL and IN (NULL) (SQC047), literal-zero division (SQC048), unfiltered UPDATE or DELETE (SQC049, opt-in), INSERT without a column list (SQC050, opt-in), NATURAL JOIN (SQC051), reversed literal BETWEEN bounds (SQC052) and aggregates or windows in WHERE, ON or GROUP BY (SQC053). * fix(rules): simplify the single-argument aggregate test for clippy
1 parent 6aef773 commit ec34b41

14 files changed

Lines changed: 1078 additions & 9 deletions

File tree

‎crates/sqlclean-cli/skill/references/docs/quickstart.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ dialect = "duckdb"
172172
select = ["SQC"]
173173
```
174174

175-
`SQC` is a prefix, so it selects all 45 built-in Rules. Three of the opt-in structure Rules now
175+
`SQC` is a prefix, so it selects all 53 built-in Rules. Three of the opt-in structure Rules now
176176
report the file:
177177

178178
```text

‎crates/sqlclean-cli/skill/references/docs/rules/overview.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ Online: https://sqlclean.io/docs/rules/overview/
1919
- Continue
2020

2121
A Rule is one check over SQL. It reports a finding at an exact position, with a message, a
22-
remediation, and sometimes a fix. SQLClean has 45 built-in Rules with `SQC###` codes, and runs
22+
remediation, and sometimes a fix. SQLClean has 53 built-in Rules with `SQC###` codes, and runs
2323
[custom Rules](../custom-rules/overview.md) written in Python with `XSQC` codes.
2424

2525
Consider a query that returns an arbitrary hundred rows:

‎crates/sqlclean-cli/skill/references/docs/rules/reference.md‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ Online: https://sqlclean.io/docs/rules/reference/
1313
- SQC005: unused CTEs
1414
- SQC018: unstable row numbers
1515
- SQC019: NULL in NOT IN
16+
- SQC046 to SQC053: queries that cannot do what they say
1617
- SQC039: inline subqueries
1718
- SQC034, SQC035 and SQC041: query structure
1819

19-
SQLClean has 45 built-in Rules. `Default` Rules run when `[rules] select` is not set; the others
20+
SQLClean has 53 built-in Rules. `Default` Rules run when `[rules] select` is not set; the others
2021
run when `select` names them by code or prefix. A Rule with a `Safe` fix is fixed by
2122
`sqlclean fix`; an `Unsafe` fix also needs `--unsafe-fixes`. See
2223
[Selecting Rules](overview.md#selecting-rules) and
@@ -71,6 +72,14 @@ Run `sqlclean rules <CODE>` for a Rule's remediation.
7172
| `SQC043` | Ranking window sorts by more expressions than max_ranking_order_by | No | — |
7273
| `SQC044` | String literal is longer than max_literal_length | No | — |
7374
| `SQC045` | JSON payload is parsed more than once in one SELECT | No | — |
75+
| `SQC046` | WITH defines the same CTE name more than once | Yes | — |
76+
| `SQC047` | CASE WHEN NULL or IN (NULL) never matches | Yes | — |
77+
| `SQC048` | Expression divides by a literal zero | Yes | — |
78+
| `SQC049` | UPDATE or DELETE has no limiting predicate and changes every row | No | — |
79+
| `SQC050` | INSERT does not list its target columns | No | — |
80+
| `SQC051` | NATURAL JOIN matches on every column the relations share | Yes | — |
81+
| `SQC052` | BETWEEN lower bound is greater than its upper bound | Yes | — |
82+
| `SQC053` | Aggregate or window function is used in WHERE, ON or GROUP BY | Yes | — |
7483

7584
`SQC999` is the [parse error](overview.md#parse-errors-sqc999) finding. It is not a
7685
selectable Rule.
@@ -225,6 +234,48 @@ error[SQC019]: NOT IN list contains NULL and can reject every row
225234
= help: Remove NULL from the list or express the anti-join with a NULL-safe NOT EXISTS.
226235
```
227236

237+
## SQC046 to SQC053: queries that cannot do what they say
238+
239+
These Rules report a query that runs differently from how it reads. None of them has a fix,
240+
because only the author knows the intended query.
241+
242+
```sql
243+
WITH paid AS (
244+
SELECT order_id, amount FROM orders WHERE amount BETWEEN 100 AND 10 -- SQC052
245+
),
246+
247+
paid AS ( -- SQC046
248+
SELECT order_id, amount / 0 AS ratio FROM orders -- SQC048
249+
)
250+
251+
SELECT
252+
customer_id,
253+
CASE region WHEN NULL THEN 'unknown' END AS region_label -- SQC047
254+
FROM paid
255+
NATURAL JOIN customers -- SQC051
256+
WHERE SUM(amount) > 100 -- SQC053
257+
GROUP BY customer_id
258+
```
259+
260+
- `SQC046` compares CTE names within one `WITH` the way the dialect resolves them, so quoted
261+
`"paid"` and `"PAID"` are different names in PostgreSQL but the same name in DuckDB. A nested
262+
`WITH` may reuse an outer name.
263+
- `SQC047` reports `CASE x WHEN NULL` and `x IN (NULL)`, which compare with `=` and so never
264+
match. `NOT IN` lists with NULL are `SQC019`.
265+
- `SQC048` reports `/`, `%`, `//` and `MOD` with a literal zero divisor. Depending on the dialect
266+
the query fails or returns NULL.
267+
- `SQC052` compares number and `DATE 'YYYY-MM-DD'` bounds exactly and skips `BETWEEN SYMMETRIC`.
268+
A reversed `NOT BETWEEN` is reported too, because it matches every non-NULL value.
269+
- `SQC053` reports aggregates in `WHERE`, `ON` and `GROUP BY`, and window functions anywhere in
270+
those clauses. Aggregates in `HAVING`, windows in `QUALIFY`, `FILTER (WHERE ...)` and
271+
subqueries pass.
272+
273+
Two write-safety Rules are opt-in, because changing every row is sometimes intended:
274+
275+
- `SQC049` reports an `UPDATE` or `DELETE` whose rows nothing limits. A `WHERE` without a column
276+
or subquery, such as `WHERE TRUE`, does not count; a join condition or a `LIMIT` does.
277+
- `SQC050` reports `INSERT` without a column list. `DEFAULT VALUES` and DuckDB's `BY NAME` pass.
278+
228279
## SQC039: inline subqueries
229280

230281
A subquery in `FROM` hides a step of the query. Name it as a CTE instead:

‎crates/sqlclean/src/sql_lint/_helpers/engine.rs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ const FINAL_CTE_NAME: LintRuleMetadata = LintRuleMetadata {
238238
remediation: "Name the last top-level CTE final and rename any other final CTE; SQC035 checks the terminal SELECT.",
239239
};
240240

241-
pub(crate) const DEFAULT_RULES: [&str; 13] = [
241+
pub(crate) const DEFAULT_RULES: [&str; 19] = [
242242
NULL_COMPARISON.code,
243243
IMPLICIT_CARTESIAN_JOIN.code,
244244
JOIN_WITHOUT_CONDITION.code,
@@ -252,9 +252,15 @@ pub(crate) const DEFAULT_RULES: [&str; 13] = [
252252
NULL_NOT_IN.code,
253253
SET_ARITY_MISMATCH.code,
254254
INLINE_QUERY_RELATION.code,
255+
crate::sql_quality::constants::DUPLICATE_CTE_NAME.code,
256+
crate::sql_quality::constants::NULL_MATCH.code,
257+
crate::sql_quality::constants::LITERAL_ZERO_DIVISION.code,
258+
crate::sql_quality::constants::NATURAL_JOIN.code,
259+
crate::sql_quality::constants::REVERSED_BETWEEN.code,
260+
crate::sql_quality::constants::MISPLACED_AGGREGATE.code,
255261
];
256262

257-
const ALL_RULE_METADATA: [&LintRuleMetadata; 45] = [
263+
const ALL_RULE_METADATA: [&LintRuleMetadata; 53] = [
258264
&NULL_COMPARISON,
259265
&IMPLICIT_CARTESIAN_JOIN,
260266
&JOIN_WITHOUT_CONDITION,
@@ -300,6 +306,14 @@ const ALL_RULE_METADATA: [&LintRuleMetadata; 45] = [
300306
&crate::sql_quality::constants::RANKING_SORT_CAP,
301307
&crate::sql_quality::constants::LONG_LITERAL,
302308
&crate::sql_quality::constants::REPEATED_JSON_PARSE,
309+
&crate::sql_quality::constants::DUPLICATE_CTE_NAME,
310+
&crate::sql_quality::constants::NULL_MATCH,
311+
&crate::sql_quality::constants::LITERAL_ZERO_DIVISION,
312+
&crate::sql_quality::constants::UNFILTERED_WRITE,
313+
&crate::sql_quality::constants::INSERT_WITHOUT_COLUMNS,
314+
&crate::sql_quality::constants::NATURAL_JOIN,
315+
&crate::sql_quality::constants::REVERSED_BETWEEN,
316+
&crate::sql_quality::constants::MISPLACED_AGGREGATE,
303317
];
304318

305319
fn is_ceremonial_cte_name(name: &str) -> bool {

0 commit comments

Comments
 (0)