Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/sqlclean-cli/skill/references/docs/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ dialect = "duckdb"
select = ["SQC"]
```

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

```text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Online: https://sqlclean.io/docs/rules/overview/
- Continue

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

Consider a query that returns an arbitrary hundred rows:
Expand Down
53 changes: 52 additions & 1 deletion crates/sqlclean-cli/skill/references/docs/rules/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ Online: https://sqlclean.io/docs/rules/reference/
- SQC005: unused CTEs
- SQC018: unstable row numbers
- SQC019: NULL in NOT IN
- SQC046 to SQC053: queries that cannot do what they say
- SQC039: inline subqueries
- SQC034, SQC035 and SQC041: query structure

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

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

## SQC046 to SQC053: queries that cannot do what they say

These Rules report a query that runs differently from how it reads. None of them has a fix,
because only the author knows the intended query.

```sql
WITH paid AS (
SELECT order_id, amount FROM orders WHERE amount BETWEEN 100 AND 10 -- SQC052
),

paid AS ( -- SQC046
SELECT order_id, amount / 0 AS ratio FROM orders -- SQC048
)

SELECT
customer_id,
CASE region WHEN NULL THEN 'unknown' END AS region_label -- SQC047
FROM paid
NATURAL JOIN customers -- SQC051
WHERE SUM(amount) > 100 -- SQC053
GROUP BY customer_id
```

- `SQC046` compares CTE names within one `WITH` the way the dialect resolves them, so quoted
`"paid"` and `"PAID"` are different names in PostgreSQL but the same name in DuckDB. A nested
`WITH` may reuse an outer name.
- `SQC047` reports `CASE x WHEN NULL` and `x IN (NULL)`, which compare with `=` and so never
match. `NOT IN` lists with NULL are `SQC019`.
- `SQC048` reports `/`, `%`, `//` and `MOD` with a literal zero divisor. Depending on the dialect
the query fails or returns NULL.
- `SQC052` compares number and `DATE 'YYYY-MM-DD'` bounds exactly and skips `BETWEEN SYMMETRIC`.
A reversed `NOT BETWEEN` is reported too, because it matches every non-NULL value.
- `SQC053` reports aggregates in `WHERE`, `ON` and `GROUP BY`, and window functions anywhere in
those clauses. Aggregates in `HAVING`, windows in `QUALIFY`, `FILTER (WHERE ...)` and
subqueries pass.

Two write-safety Rules are opt-in, because changing every row is sometimes intended:

- `SQC049` reports an `UPDATE` or `DELETE` whose rows nothing limits. A `WHERE` without a column
or subquery, such as `WHERE TRUE`, does not count; a join condition or a `LIMIT` does.
- `SQC050` reports `INSERT` without a column list. `DEFAULT VALUES` and DuckDB's `BY NAME` pass.

## SQC039: inline subqueries

A subquery in `FROM` hides a step of the query. Name it as a CTE instead:
Expand Down
18 changes: 16 additions & 2 deletions crates/sqlclean/src/sql_lint/_helpers/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ const FINAL_CTE_NAME: LintRuleMetadata = LintRuleMetadata {
remediation: "Name the last top-level CTE final and rename any other final CTE; SQC035 checks the terminal SELECT.",
};

pub(crate) const DEFAULT_RULES: [&str; 13] = [
pub(crate) const DEFAULT_RULES: [&str; 19] = [
NULL_COMPARISON.code,
IMPLICIT_CARTESIAN_JOIN.code,
JOIN_WITHOUT_CONDITION.code,
Expand All @@ -252,9 +252,15 @@ pub(crate) const DEFAULT_RULES: [&str; 13] = [
NULL_NOT_IN.code,
SET_ARITY_MISMATCH.code,
INLINE_QUERY_RELATION.code,
crate::sql_quality::constants::DUPLICATE_CTE_NAME.code,
crate::sql_quality::constants::NULL_MATCH.code,
crate::sql_quality::constants::LITERAL_ZERO_DIVISION.code,
crate::sql_quality::constants::NATURAL_JOIN.code,
crate::sql_quality::constants::REVERSED_BETWEEN.code,
crate::sql_quality::constants::MISPLACED_AGGREGATE.code,
];

const ALL_RULE_METADATA: [&LintRuleMetadata; 45] = [
const ALL_RULE_METADATA: [&LintRuleMetadata; 53] = [
&NULL_COMPARISON,
&IMPLICIT_CARTESIAN_JOIN,
&JOIN_WITHOUT_CONDITION,
Expand Down Expand Up @@ -300,6 +306,14 @@ const ALL_RULE_METADATA: [&LintRuleMetadata; 45] = [
&crate::sql_quality::constants::RANKING_SORT_CAP,
&crate::sql_quality::constants::LONG_LITERAL,
&crate::sql_quality::constants::REPEATED_JSON_PARSE,
&crate::sql_quality::constants::DUPLICATE_CTE_NAME,
&crate::sql_quality::constants::NULL_MATCH,
&crate::sql_quality::constants::LITERAL_ZERO_DIVISION,
&crate::sql_quality::constants::UNFILTERED_WRITE,
&crate::sql_quality::constants::INSERT_WITHOUT_COLUMNS,
&crate::sql_quality::constants::NATURAL_JOIN,
&crate::sql_quality::constants::REVERSED_BETWEEN,
&crate::sql_quality::constants::MISPLACED_AGGREGATE,
];

fn is_ceremonial_cte_name(name: &str) -> bool {
Expand Down
Loading
Loading