From f1e9268bbdf8245eb0fab39cda2ac2f2c89f9a17 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 9 Nov 2026 03:50:54 +0000 Subject: [PATCH 1/2] fix(format): lay out a placeholder that stands for a CTE's whole query A host writes one interpolation placeholder for a macro that expands to a whole query, as in `headers AS (__sqlclean_interpolation_6__)`. SQLQuack cannot parse a bare name as a query, so the file was declined. The formatter now writes `SELECT ` before such a placeholder, lays the SQL out, and takes the `SELECT` back out, checking that every token is unchanged. A SELECT the author wrote before a placeholder is kept. --- .../src/sql_lint/_helpers/cte_macros.rs | 128 +++++++++++++++--- .../src/sql_lint/_helpers/formatter.rs | 19 ++- .../src/sql_lint/tests/test_layout_rules.rs | 21 +++ 3 files changed, 150 insertions(+), 18 deletions(-) diff --git a/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs b/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs index de06f7a..c55eccb 100644 --- a/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs +++ b/crates/sqlclean/src/sql_lint/_helpers/cte_macros.rs @@ -10,6 +10,8 @@ use crate::constants::INTERPOLATION_PLACEHOLDER_PREFIX; /// The separator token taken out with a placeholder. const COMMA: &str = ","; const RESTORE_FAILURE: &str = "native formatter could not restore CTE macro placeholders"; +/// What is written before a placeholder standing for a whole query, so it parses as one. +const QUERY_WRAPPER: &str = "SELECT "; /// SQL with its CTE macro placeholders taken out, and the groups to put back. pub(super) struct WithoutCteMacros { @@ -141,25 +143,119 @@ impl WithoutCteMacros { cursor = lead_end; } restored.push_str(&formatted[cursor..]); - let restored_tokens = sqlquack_syntax::tokenize(&restored, dialect) + restored_with_tokens(restored, &self.original, dialect) + } +} + +/// SQL whose placeholders standing for a whole parenthesised query were written as `SELECT `. +pub(super) struct WrappedQueryPlaceholders { + pub sql: String, + placeholders: Vec, + /// The significant and comment texts of the original SQL, in order. + original: Vec<(bool, String)>, +} + +/// Write `SELECT ` before each placeholder that is the whole query in `AS ()`, such as a CTE whose body a macro writes, which SQLQuack cannot parse as a query; `None` when there are none. +pub(super) fn wrap_query_placeholders( + sql: &str, + dialect: Dialect, +) -> Option { + if !sql.contains(INTERPOLATION_PLACEHOLDER_PREFIX) { + return None; + } + let Ok(tokens) = sqlquack_syntax::tokenize(sql, dialect) else { + return None; + }; + let significant: Vec<&LosslessToken> = tokens + .iter() + .filter(|token| matches!(token.kind, TokenKind::Significant(_))) + .collect(); + let is = |position: usize, kind: TokenType| { + significant + .get(position) + .is_some_and(|token| token.kind == TokenKind::Significant(kind)) + }; + let mut wrapped = String::with_capacity(sql.len() + QUERY_WRAPPER.len()); + let mut placeholders: Vec = Vec::new(); + let mut cursor = 0_usize; + for (position, token) in significant.iter().enumerate().skip(2) { + let text = token.text(sql); + if text.starts_with(INTERPOLATION_PLACEHOLDER_PREFIX) + && is(position - 2, TokenType::As) + && is(position - 1, TokenType::LParen) + && is(position + 1, TokenType::RParen) + { + wrapped.push_str(&sql[cursor..token.start]); + wrapped.push_str(QUERY_WRAPPER); + cursor = token.start; + placeholders.push(text.to_string()); + } + } + if placeholders.is_empty() { + return None; + } + wrapped.push_str(&sql[cursor..]); + Some(WrappedQueryPlaceholders { + sql: wrapped, + placeholders, + original: token_texts(sql, &tokens), + }) +} + +impl WrappedQueryPlaceholders { + /// Take the `SELECT` written before each wrapped placeholder back out of `formatted`. + pub(super) fn restore(&self, formatted: &str, dialect: Dialect) -> Result { + let tokens = sqlquack_syntax::tokenize(formatted, dialect) .map_err(|_| RESTORE_FAILURE.to_string())?; - let restored_texts = token_texts(&restored, &restored_tokens); - let same_tokens = restored_texts.len() == self.original.len() - && restored_texts.iter().zip(&self.original).all( - |((comment, text), (original_comment, original))| { - comment == original_comment - && if *comment { - text == original - } else { - text.eq_ignore_ascii_case(original) - } - }, - ); - if !same_tokens { - return Err(RESTORE_FAILURE.to_string()); + let significant: Vec<&LosslessToken> = tokens + .iter() + .filter(|token| matches!(token.kind, TokenKind::Significant(_))) + .collect(); + let mut restored = String::with_capacity(formatted.len()); + let mut cursor = 0_usize; + for pair in significant.windows(2) { + let [select, placeholder] = pair else { + continue; + }; + if select.kind == TokenKind::Significant(TokenType::Select) + && self + .placeholders + .iter() + .any(|wrapped| wrapped == placeholder.text(formatted)) + { + restored.push_str(&formatted[cursor..select.start]); + cursor = placeholder.start; + } } - Ok(restored) + restored.push_str(&formatted[cursor..]); + restored_with_tokens(restored, &self.original, dialect) + } +} + +/// `restored`, when its significant and comment tokens are `original`'s, keywords compared without case. +fn restored_with_tokens( + restored: String, + original: &[(bool, String)], + dialect: Dialect, +) -> Result { + let restored_tokens = + sqlquack_syntax::tokenize(&restored, dialect).map_err(|_| RESTORE_FAILURE.to_string())?; + let restored_texts = token_texts(&restored, &restored_tokens); + let same_tokens = restored_texts.len() == original.len() + && restored_texts.iter().zip(original).all( + |((comment, text), (original_comment, original))| { + comment == original_comment + && if *comment { + text == original + } else { + text.eq_ignore_ascii_case(original) + } + }, + ); + if !same_tokens { + return Err(RESTORE_FAILURE.to_string()); } + Ok(restored) } /// Positions, among significant tokens, of placeholders that stand where a CTE would. diff --git a/crates/sqlclean/src/sql_lint/_helpers/formatter.rs b/crates/sqlclean/src/sql_lint/_helpers/formatter.rs index 6284a55..e61413e 100644 --- a/crates/sqlclean/src/sql_lint/_helpers/formatter.rs +++ b/crates/sqlclean/src/sql_lint/_helpers/formatter.rs @@ -3,7 +3,7 @@ use sqlquack_format::{FormatDialect, FormatOptions}; use sqlquack_syntax::Dialect as SqlQuackDialect; use crate::constants::INTERPOLATION_PLACEHOLDER_PREFIX; -use crate::sql_lint::_helpers::cte_macros::take_out_cte_macros; +use crate::sql_lint::_helpers::cte_macros::{take_out_cte_macros, wrap_query_placeholders}; use crate::sql_lint::constants::{ BIGQUERY_DIALECT, DUCKDB_DIALECT, LINT_API_VERSION, POSTGRES_DIALECTS, SNOWFLAKE_DIALECT, TSQL_DIALECTS, @@ -114,7 +114,7 @@ fn sqlquack_layout(sql: &str, options: &FormatOptions) -> Result .map_err(|refusal| refusal.reason) } -/// Lay out `sql` with SQLQuack, taking CTE macro placeholders out first and putting them back where `macros` names the shared tokenizer's dialect; otherwise SQLQuack refuses a placeholder it cannot parse. +/// Lay out `sql` with SQLQuack, first making placeholders it cannot parse parseable and then restoring them, where `macros` names the shared tokenizer's dialect: a placeholder standing for a whole query is wrapped as `SELECT `, and CTE macro placeholders are taken out. fn format_around_cte_macros( sql: &str, macros: Option, @@ -123,6 +123,21 @@ fn format_around_cte_macros( let Some(dialect) = macros else { return sqlquack_layout(sql, options); }; + match wrap_query_placeholders(sql, dialect) { + None => format_around_cte_lists(sql, dialect, options), + Some(wrapped) => wrapped.restore( + &format_around_cte_lists(&wrapped.sql, dialect, options)?, + dialect, + ), + } +} + +/// Lay out `sql` with SQLQuack, taking CTE macro placeholders out first and putting them back. +fn format_around_cte_lists( + sql: &str, + dialect: SqlQuackDialect, + options: &FormatOptions, +) -> Result { match take_out_cte_macros(sql, dialect)? { None => sqlquack_layout(sql, options), Some(without) => without.put_back(&sqlquack_layout(&without.sql, options)?, dialect), diff --git a/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs b/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs index c6bf7f2..2c9d278 100644 --- a/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs +++ b/crates/sqlclean/src/sql_lint/tests/test_layout_rules.rs @@ -121,6 +121,27 @@ fn given_clause_layout_rules_when_formatting_then_each_rule_applies_idempotently line_width: 100, expected_sql: "WITH a AS (\n SELECT 1 AS x\n),\n\n__sqlclean_interpolation_0__\n\n__sqlclean_interpolation_1__\n\npaid AS (\n SELECT *\n FROM a\n)\n\nSELECT *\nFROM paid", }, + test_types::LayoutRuleTestCase { + description: "a placeholder standing for a CTE's whole query keeps its place", + dialect: "snowflake", + sql: "WITH headers AS (__sqlclean_interpolation_0__), paid AS (SELECT * FROM headers) SELECT * FROM paid", + line_width: 100, + expected_sql: "WITH headers AS (\n __sqlclean_interpolation_0__\n),\n\npaid AS (\n SELECT *\n FROM headers\n)\n\nSELECT *\nFROM paid", + }, + test_types::LayoutRuleTestCase { + description: "a query placeholder in a test's input CTE keeps its place before the ceremonial select", + dialect: "snowflake", + sql: "WITH __ref__orders AS (\n __sqlclean_interpolation_0__\n),\n\n__expected__paid AS (SELECT 1 AS id)\nSELECT 1", + line_width: 100, + expected_sql: "WITH __ref__orders AS (\n __sqlclean_interpolation_0__\n),\n\n__expected__paid AS (\n SELECT 1 AS id\n)\n\nSELECT 1", + }, + test_types::LayoutRuleTestCase { + description: "an authored SELECT before a placeholder in a CTE is kept", + dialect: "snowflake", + sql: "WITH a AS (SELECT __sqlclean_interpolation_0__), b AS (__sqlclean_interpolation_1__) SELECT * FROM a", + line_width: 100, + expected_sql: "WITH a AS (\n SELECT __sqlclean_interpolation_0__\n),\n\nb AS (\n __sqlclean_interpolation_1__\n)\n\nSELECT *\nFROM a", + }, test_types::LayoutRuleTestCase { description: "an alias and comma that push a call over the width break the call", dialect: "snowflake", From b55cb1b146e00d06c58c6f00c0365808d6debe47 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 9 Nov 2026 09:21:17 +0000 Subject: [PATCH 2/2] build(deps): pin SQLQuack 804864d2 to match SQLBuild Every SQLQuack crate moves to the same rev the SQLBuild cutover pins, so a build that uses both has one copy of SQLQuack. SQLQuack now fingerprints SQL Server with its own tokenizer, so a bracketed name such as `[o]` is one token. The fingerprint of a SQL Server query with bracketed names changes once from SQLClean 0.5.0 and 0.6.0, back to the value SQLBuild 0.204.0 stores: the golden row moves from f1525ee2 to f4fac3ed. Every other golden fingerprint is unchanged. --- Cargo.lock | 20 ++++++++++--------- Cargo.toml | 10 +++++----- .../tests/test_query_fingerprint.rs | 7 +++++-- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3cdff57..c134d60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -612,17 +612,19 @@ dependencies = [ [[package]] name = "sqlquack-fingerprint" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "sha2", "sqlquack-syntax", + "sqlquack-syntax-bigquery", "sqlquack-syntax-postgres", + "sqlquack-syntax-sqlserver", ] [[package]] name = "sqlquack-format" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "serde_json", "sqlquack-syntax", @@ -634,7 +636,7 @@ dependencies = [ [[package]] name = "sqlquack-lint" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "rayon", "rustc-hash", @@ -649,7 +651,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "serde", "serde_json", @@ -662,7 +664,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax-bigquery" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "sqlquack-syntax", "stacker", @@ -671,7 +673,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax-databricks" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "sqlquack-syntax", "stacker", @@ -680,7 +682,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax-derive" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "proc-macro2", "quote", @@ -690,7 +692,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax-postgres" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "sqlquack-syntax", "stacker", @@ -699,7 +701,7 @@ dependencies = [ [[package]] name = "sqlquack-syntax-sqlserver" version = "0.2.0" -source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=33e4384967e31d44c2d453640c45ccd222320c93#33e4384967e31d44c2d453640c45ccd222320c93" +source = "git+https://github.com/sqlstack-io/sqlquack.git?rev=804864d2073ef869cbfe4a9334f7e48b20072326#804864d2073ef869cbfe4a9334f7e48b20072326" dependencies = [ "sqlquack-syntax", "stacker", diff --git a/Cargo.toml b/Cargo.toml index a064a71..0d0b54a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,11 +30,11 @@ sha2 = "0.10" similar = "2" sqlclean = { path = "crates/sqlclean" } # SQLQuack's formatter and lint engine, and the tokens and trees they read, which replace Polyglot. -sqlquack-fingerprint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "33e4384967e31d44c2d453640c45ccd222320c93" } -sqlquack-format = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "33e4384967e31d44c2d453640c45ccd222320c93" } -sqlquack-lint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "33e4384967e31d44c2d453640c45ccd222320c93" } -sqlquack-syntax = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "33e4384967e31d44c2d453640c45ccd222320c93" } -sqlquack-syntax-sqlserver = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "33e4384967e31d44c2d453640c45ccd222320c93" } +sqlquack-fingerprint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "804864d2073ef869cbfe4a9334f7e48b20072326" } +sqlquack-format = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "804864d2073ef869cbfe4a9334f7e48b20072326" } +sqlquack-lint = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "804864d2073ef869cbfe4a9334f7e48b20072326" } +sqlquack-syntax = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "804864d2073ef869cbfe4a9334f7e48b20072326" } +sqlquack-syntax-sqlserver = { git = "https://github.com/sqlstack-io/sqlquack.git", rev = "804864d2073ef869cbfe4a9334f7e48b20072326" } [workspace.lints.rust] unsafe_code = "forbid" diff --git a/crates/sqlclean/src/sql_tokens/tests/test_query_fingerprint.rs b/crates/sqlclean/src/sql_tokens/tests/test_query_fingerprint.rs index 5957db8..59b6866 100644 --- a/crates/sqlclean/src/sql_tokens/tests/test_query_fingerprint.rs +++ b/crates/sqlclean/src/sql_tokens/tests/test_query_fingerprint.rs @@ -48,11 +48,14 @@ fn given_sqlbuild_golden_queries_when_fingerprinting_then_hashes_match_sqlbuild( sql: "SELECT /*+ BROADCAST(o) */ o.id FROM `catalog`.`orders` AS o", expected_fingerprint: "7cd96f42bcb764136a7f423181302ba13cead02bb6cfb7382502a3a2aa4107c3", }, + // D651, D695: SQL Server is tokenized natively, so a bracketed name such as `[o]` is one + // token. This is SQLBuild 0.204.0's digest; SQLClean 0.5.0 and 0.6.0 gave the generic + // tokens' f1525ee2… instead. test_types::FingerprintGoldenTestCase { - description: "SQL Server bracket identifiers, as generic tokens", + description: "SQL Server bracket identifiers, as SQL Server tokens", dialect: "sqlserver", sql: "SELECT TOP 10 [o].[id], N'order' AS kind FROM [dbo].[orders] AS o", - expected_fingerprint: "f1525ee25924f9402677bc2cbd96f7066745948da73671d13fac2b8da03cbf0c", + expected_fingerprint: "f4fac3edb7682bbeddf4f73cc199493d45f1419bc8e2cd1d972fead2d67e6b71", }, ]; for test_case in &test_cases {