diff --git a/crates/sqlclean/src/sql_lint/_helpers/engine.rs b/crates/sqlclean/src/sql_lint/_helpers/engine.rs index 93b4225..2a9d874 100644 --- a/crates/sqlclean/src/sql_lint/_helpers/engine.rs +++ b/crates/sqlclean/src/sql_lint/_helpers/engine.rs @@ -1876,11 +1876,12 @@ fn null_comparison_spans(tokens: &[Token]) -> Vec { TokenType::Gte, ]; let depths = token_depths(tokens); + let assignments: Vec = assignment_operators(tokens, &depths); tokens .iter() .enumerate() .filter(|(_, token)| comparison_types.contains(&token.token_type)) - .filter(|(index, _)| !is_assignment_operator(tokens, &depths, *index)) + .filter(|(index, _)| !assignments[*index]) .filter(|(index, _)| { adjacent_null(tokens, *index, false) || adjacent_null(tokens, *index, true) }) @@ -1888,22 +1889,27 @@ fn null_comparison_spans(tokens: &[Token]) -> Vec { .collect() } -fn is_assignment_operator(tokens: &[Token], depths: &[usize], operator_index: usize) -> bool { - let depth = depths[operator_index]; - for index in (0..operator_index).rev() { - if depths[index] != depth { - continue; +/// For each token, whether its nearest same-depth clause opener is `SET`, found in one forward pass. +fn assignment_operators(tokens: &[Token], depths: &[usize]) -> Vec { + let mut in_set: Vec = Vec::new(); + let mut assignments: Vec = Vec::with_capacity(tokens.len()); + for (token, &depth) in tokens.iter().zip(depths) { + if in_set.len() <= depth { + in_set.resize(depth + 1, false); } - match tokens[index].token_type { - TokenType::Where | TokenType::Having | TokenType::Qualify | TokenType::On => { - return false; - } - TokenType::Set => return true, - TokenType::Select | TokenType::Semicolon => return false, + assignments.push(in_set[depth]); + match token.token_type { + TokenType::Set => in_set[depth] = true, + TokenType::Where + | TokenType::Having + | TokenType::Qualify + | TokenType::On + | TokenType::Select + | TokenType::Semicolon => in_set[depth] = false, _ => {} } } - false + assignments } fn adjacent_null(tokens: &[Token], operator_index: usize, forward: bool) -> bool { diff --git a/crates/sqlclean/src/sql_quality/_helpers/safety.rs b/crates/sqlclean/src/sql_quality/_helpers/safety.rs index 6615bf3..49d638c 100644 --- a/crates/sqlclean/src/sql_quality/_helpers/safety.rs +++ b/crates/sqlclean/src/sql_quality/_helpers/safety.rs @@ -702,6 +702,7 @@ fn ends_clause(significant: &Significant<'_>, index: usize, comma_ends: bool) -> | TokenType::Cross | TokenType::Natural | TokenType::Set + | TokenType::Using ), None => true, } diff --git a/crates/sqlclean/src/sql_quality/tests/test_safety_rules.rs b/crates/sqlclean/src/sql_quality/tests/test_safety_rules.rs index 74b5156..b04263d 100644 --- a/crates/sqlclean/src/sql_quality/tests/test_safety_rules.rs +++ b/crates/sqlclean/src/sql_quality/tests/test_safety_rules.rs @@ -171,6 +171,20 @@ fn given_safety_rule_inputs_when_linting_then_exactly_the_mistakes_are_reported( rule: "SQLCLEAN053", expected_anchors: &[], }, + SafetyRuleTestCase { + description: "DuckDB PIVOT USING aggregates end the pivot ON clause", + dialect: "duckdb", + sql: "PIVOT orders ON status IN ('placed') USING MAX(amount), SUM(amount) GROUP BY customer_id", + rule: "SQLCLEAN053", + expected_anchors: &[], + }, + SafetyRuleTestCase { + description: "a join ON aggregate is still reported beside a pivot", + dialect: "duckdb", + sql: "SELECT o.id FROM orders AS o JOIN (PIVOT items ON kind USING MAX(qty)) AS p ON p.id = o.id AND MIN(o.amount) > 0", + rule: "SQLCLEAN053", + expected_anchors: &["MIN(o.amount)"], + }, SafetyRuleTestCase { description: "multi-argument MAX is the scalar function in SQLite", dialect: "sqlite",