From c7bd825108d2a015a21727b69e914ffb0460c12c Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:49:40 +0000 Subject: [PATCH 1/8] fix(syntax): parse keyword lambda parameters and nested parenthesised set operations DuckDB takes a keyword such as row or top as a LAMBDA parameter name, and ((SELECT 1) UNION ALL (SELECT 2)) UNION ALL (SELECT 3) nests a set operation in parentheses. Converted SQL uses both, so the DuckDB profile parses them. --- .../parser/_helpers/expressions/grouping.rs | 10 +++- .../src/parser/_helpers/expressions/paths.rs | 12 ++++- .../tests/parser/test_fixes.rs | 49 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/grouping.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/grouping.rs index dc96564cc..9b8f82496 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/grouping.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/grouping.rs @@ -499,14 +499,20 @@ impl Parser { } } - /// Parses a lambda: `x -> x + 1`, `(x, y) -> ...` or DuckDB `LAMBDA x : x + 1`. + /// Parses a lambda: `x -> x + 1`, `(x, y) -> ...` or DuckDB `LAMBDA row, x : x + 1`, whose parameter may be a keyword. pub(crate) fn parse_lambda(&mut self) -> Result> { let start_index = self.current; if self.match_token(TokenType::Lambda) { let mut params: Vec = Vec::new(); loop { - if self.is_identifier_token() { + let word = self + .peek() + .text + .starts_with(|first: char| first.is_ascii_alphabetic() || first == '_'); + let keyword_name = word + && (self.check_next(TokenType::Comma) || self.check_next(TokenType::Colon)); + if self.is_identifier_token() || keyword_name { let token = self.advance()?; let quoted = token.token_type == TokenType::QuotedIdentifier; params.push(identifier_from_token(token, quoted)); diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs index 0edc87235..86d2b25b0 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs @@ -827,6 +827,7 @@ impl Parser { self.maybe_parse_subscript(result) } + /// Parses `((...` past its first parenthesis, where a parenthesised set operation may start a longer one. fn parse_doubly_parenthesized( &mut self, lparen_comments: Vec, @@ -874,8 +875,15 @@ impl Parser { || self.check(TokenType::Intersect) || self.check(TokenType::Except)) { - if let Expression::Subquery(subq) = &result { - let set_result = self.parse_set_operation(subq.this.clone())?; + let left = match &result { + Expression::Subquery(subq) => Some(subq.this.clone()), + Expression::Union(_) | Expression::Intersect(_) | Expression::Except(_) => { + Some(result.clone()) + } + _ => None, + }; + if let Some(left) = left { + let set_result = self.parse_set_operation(left)?; let order_by = if self.check(TokenType::Order) { self.expect_token(TokenType::Order)?; diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs index 1d7e072f1..c2f8315e5 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -1874,3 +1874,52 @@ fn given_a_time_travel_clause_when_parsing_snowflake_then_its_parentheses_close( ); } } + +#[test] +fn given_a_keyword_lambda_parameter_when_parsing_duckdb_then_it_names_the_parameter() { + let test_cases = [ + StatementTestCase { + description: "LAMBDA top: names its parameter top", + sql: "SELECT list_transform([1], lambda top: top + 1)", + pointer: "/0/select/expressions/0/function/args/1/lambda/parameters/0/name", + expected_value: json!("top"), + }, + StatementTestCase { + description: "LAMBDA top: has one parameter", + sql: "SELECT list_transform([1], lambda top: top + 1)", + pointer: "/0/select/expressions/0/function/args/1/lambda/parameters/1", + expected_value: json!(null), + }, + StatementTestCase { + description: "LAMBDA row, j: names a keyword first parameter row", + sql: "SELECT list_reduce([1, 2], lambda row, j: row + j)", + pointer: "/0/select/expressions/0/function/args/1/lambda/parameters/0/name", + expected_value: json!("row"), + }, + StatementTestCase { + description: "LAMBDA row, j: names its second parameter j", + sql: "SELECT list_reduce([1, 2], lambda row, j: row + j)", + pointer: "/0/select/expressions/0/function/args/1/lambda/parameters/1/name", + expected_value: json!("j"), + }, + StatementTestCase { + description: "the body reads the keyword parameter as a column", + sql: "SELECT list_reduce([1, 2], lambda row, j: row + j)", + pointer: + "/0/select/expressions/0/function/args/1/lambda/body/add/left/column/name/name", + expected_value: json!("row"), + }, + ]; + + for test_case in test_cases { + assert_eq!( + statements(test_case.sql, Dialect::DuckDB, CallMode::Faithful) + .pointer(test_case.pointer) + .cloned() + .unwrap_or_default(), + test_case.expected_value, + "{}", + test_case.description + ); + } +} From 60d2692e23d75b068bad08e509c47e6f21aa7f69 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:49:40 +0000 Subject: [PATCH 2/8] feat(format): keep token case on request and lay out converted DuckDB SQL FormatOptions gains keep_case, which leaves every token's case as written. The DuckDB profile now lays out what convert emits: CREATE OR REPLACE stays together when a long macro wraps, a lambda body that starts with a parenthesis stays an expression, /, // and % are spaced and broken as binary operators, a keyword lambda parameter keeps its spaced colon, and a group a broken operator leaves open is indented with it. --- .../duckdb/profile/tests/test_format_sql.rs | 49 +++++++++++++++++++ .../src/format/_helpers/guide.rs | 15 ++++-- .../src/format/_helpers/identifiers.rs | 11 ++++- .../src/format/_helpers/pipeline.rs | 2 +- .../src/format/_helpers/printed_layout.rs | 20 +++++++- .../src/format/_helpers/token_layout.rs | 5 +- .../sqlquack-format/src/format/constants.rs | 9 ++-- crates/sqlquack-format/src/format/models.rs | 5 +- 8 files changed, 102 insertions(+), 14 deletions(-) diff --git a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs index 500505d5a..78590a45d 100644 --- a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs @@ -87,6 +87,55 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re } } +#[test] +fn given_constructs_sqlquack_converts_to_when_formatting_then_they_are_laid_out() { + let test_cases = [ + test_types::FormatSqlTestCase { + description: "integer division, modulo and division written touching are spaced as binary operators", + sql: "select a//b, a%b, x[1]/2 from t", + line_width: None, + dialect: FormatDialect::DuckDB, + expected_outcome: Ok("SELECT\n a // b,\n a % b,\n x[1] / 2\nFROM t"), + }, + test_types::FormatSqlTestCase { + description: "lambda parameters spelled like keywords parse, and their colon stays spaced", + sql: "select list_transform([1, 2], lambda row, top: row + top)", + line_width: None, + dialect: FormatDialect::DuckDB, + expected_outcome: Ok("SELECT LIST_TRANSFORM([1, 2], LAMBDA row, top: row + top)"), + }, + test_types::FormatSqlTestCase { + description: "a parenthesised set operation starts a longer one", + sql: "select x in (((select 1) union all (select 2)) union all (select 3)) from t", + line_width: None, + dialect: FormatDialect::DuckDB, + expected_outcome: Ok( + "SELECT\n x IN ((\n (\n SELECT 1\n ) UNION ALL (\n SELECT 2\n )\n ) UNION ALL (\n SELECT 3\n ))\nFROM t", + ), + }, + test_types::FormatSqlTestCase { + description: "a wrapped macro keeps CREATE OR REPLACE together and a lambda body that starts with a parenthesis inline", + sql: "create or replace macro m(a, b) as list_transform([a], lambda r: (a // b) * r + a // b)[1]", + line_width: Some(40), + dialect: FormatDialect::DuckDB, + expected_outcome: Ok( + "CREATE OR REPLACE macro m(\n a,\n b\n) AS list_transform(\n [a],\n LAMBDA r: (a // b) * r + a // b\n)[1]", + ), + }, + ]; + for test_case in test_cases { + assert_eq!( + format_outcome(test_case.sql, test_case.dialect, test_case.line_width), + test_case + .expected_outcome + .map(str::to_string) + .map_err(str::to_string), + "{}", + test_case.description + ); + } +} + #[test] fn given_deeply_nested_sql_when_formatting_then_it_is_refused_or_laid_out_without_exhausting_the_stack() { diff --git a/crates/sqlquack-format/src/format/_helpers/guide.rs b/crates/sqlquack-format/src/format/_helpers/guide.rs index 0b22578f4..e77e6a810 100644 --- a/crates/sqlquack-format/src/format/_helpers/guide.rs +++ b/crates/sqlquack-format/src/format/_helpers/guide.rs @@ -489,13 +489,14 @@ impl<'a> Guide<'a> { }) } - /// Whether the `:` at `index` ends the parameters of a DuckDB `LAMBDA x, y :`. + /// Whether the `:` at `index` ends the parameters of a DuckDB `LAMBDA x, y :`, which may be keywords. fn lambda_colon(&self, index: usize) -> bool { let parameters = (0..index).rev().find(|&at| { - !matches!( + !(matches!( self.lexemes[at].kind, TokenType::Var | TokenType::Identifier | TokenType::Comma - ) || self.is(at, LAMBDA_KEYWORD) + ) || self.is_word(at)) + || self.is(at, LAMBDA_KEYWORD) }); parameters.is_some_and(|at| self.is(at, LAMBDA_KEYWORD)) } @@ -1314,6 +1315,14 @@ impl<'a> Guide<'a> { self.call(previous, opener, closer, indent); } else if (self.is_word(previous) && !self.is_any(previous, &EXPRESSION_LEADERS)) || self.in_run[previous] + || (self.lexemes[previous].kind == TokenType::Colon + && self.lambda_colon(previous) + && self.kind(closer + 1).is_some_and(|next| { + !matches!( + next, + TokenType::Comma | TokenType::RParen | TokenType::RBracket + ) + })) { self.expr(opener + 1, closer - 1, indent); } else { diff --git a/crates/sqlquack-format/src/format/_helpers/identifiers.rs b/crates/sqlquack-format/src/format/_helpers/identifiers.rs index a440d22cc..176f49e85 100644 --- a/crates/sqlquack-format/src/format/_helpers/identifiers.rs +++ b/crates/sqlquack-format/src/format/_helpers/identifiers.rs @@ -16,6 +16,9 @@ const PROJECTION_FIELD: &str = "expressions"; const SPAN_FIELD: &str = "span"; const LEFT_FIELD: &str = "left"; const RIGHT_FIELD: &str = "right"; +/// The operand fields of a binary node read as a function, such as DuckDB's `//`. +const FUNCTION_LEFT_FIELD: &str = "this"; +const FUNCTION_RIGHT_FIELD: &str = "expression"; const COLUMN_ALIASES_FIELD: &str = "column_aliases"; /// The field of a serialised UPDATE that holds its `column = value` pairs. const SET_FIELD: &str = "set"; @@ -316,8 +319,12 @@ fn binary(value: &Value) -> Option { span: node .get(SPAN_FIELD) .and_then(|span| Some((span_offset(span, "start")?, span_offset(span, "end")?))), - left_end: operand_span(LEFT_FIELD).and_then(|span| span_offset(span, "end")), - right_start: operand_span(RIGHT_FIELD).and_then(|span| span_offset(span, "start")), + left_end: operand_span(LEFT_FIELD) + .or_else(|| operand_span(FUNCTION_LEFT_FIELD)) + .and_then(|span| span_offset(span, "end")), + right_start: operand_span(RIGHT_FIELD) + .or_else(|| operand_span(FUNCTION_RIGHT_FIELD)) + .and_then(|span| span_offset(span, "start")), before_last: false, }) } diff --git a/crates/sqlquack-format/src/format/_helpers/pipeline.rs b/crates/sqlquack-format/src/format/_helpers/pipeline.rs index e5fbf05cc..e950fb643 100644 --- a/crates/sqlquack-format/src/format/_helpers/pipeline.rs +++ b/crates/sqlquack-format/src/format/_helpers/pipeline.rs @@ -102,7 +102,7 @@ fn print_layout( options: &FormatOptions, profile: &FormatProfile, ) -> Result { - let print = print_authored_tokens(authored, oracle)?; + let print = print_authored_tokens(authored, oracle, options.keep_case)?; let Some(mut sql) = PrintedSql::new(&print.sql, &print.tokens, &options.token_widths) else { return Ok(print); }; diff --git a/crates/sqlquack-format/src/format/_helpers/printed_layout.rs b/crates/sqlquack-format/src/format/_helpers/printed_layout.rs index 272d7853b..5f7fea3a1 100644 --- a/crates/sqlquack-format/src/format/_helpers/printed_layout.rs +++ b/crates/sqlquack-format/src/format/_helpers/printed_layout.rs @@ -20,7 +20,7 @@ const SINGLE_ITEM_CLAUSES: [&str; 4] = ["SELECT", "WHERE", "QUALIFY", "HAVING"]; const SELECT_MODIFIERS: [&str; 10] = [ "DISTINCT", "ALL", "TOP", "PERCENT", "WITH", "TIES", "ON", "AS", "STRUCT", "VALUE", ]; -const LEADING_OPERATORS: [&str; 8] = ["+", "-", "*", "/", "%", "||", "AND", "OR"]; +const LEADING_OPERATORS: [&str; 9] = ["+", "-", "*", "/", "//", "%", "||", "AND", "OR"]; const CHAIN_KEYWORDS: [&str; 2] = ["AND", "OR"]; const BETWEEN_BOUNDARIES: [&str; 12] = [ "AND", "OR", "WHEN", "THEN", "ELSE", "WHERE", "ON", "HAVING", "QUALIFY", "SELECT", ",", "NOT", @@ -993,7 +993,7 @@ impl PrintedSql<'_> { } } - /// Break before every operator of `operators` at the line's outermost depth. + /// Break before every operator of `operators` at the line's outermost depth, indenting a group it leaves open. fn break_before(&mut self, line: &Line, operators: &[&str], extra_indent: usize) -> bool { let depth = self.line_depth(line); let mut between = false; @@ -1015,6 +1015,7 @@ impl PrintedSql<'_> { && operators.contains(&upper.as_str()) && self.gaps[index] == SINGLE_SPACE && ends_operand(&self.tokens[index - 1]) + && !self.is(index - 1, "CREATE") { breaks.push(index); } @@ -1023,6 +1024,21 @@ impl PrintedSql<'_> { for index in &breaks { self.gaps[*index] = indentation.clone(); } + if extra_indent > 0 + && let Some(&first) = breaks.first() + { + let mut continued: Option = None; + for index in first..=line.last { + if let Some(closer) = self.partners[index] + && closer > line.last + { + continued = continued.max(Some(closer)); + } + } + if let Some(closer) = continued { + self.indent_lines(line.last + 1, closer, extra_indent); + } + } !breaks.is_empty() } diff --git a/crates/sqlquack-format/src/format/_helpers/token_layout.rs b/crates/sqlquack-format/src/format/_helpers/token_layout.rs index e89131328..cc05a71f3 100644 --- a/crates/sqlquack-format/src/format/_helpers/token_layout.rs +++ b/crates/sqlquack-format/src/format/_helpers/token_layout.rs @@ -143,10 +143,11 @@ fn keyed_authored(lexemes: &[Lexeme], statement_starts: &[usize]) -> Vec .collect() } -/// Print every authored token in order, taking only whitespace and keyword case from `oracle`. +/// Print every authored token in order, taking only whitespace from `oracle`, and keyword case too unless `keep_case`. pub(crate) fn print_authored_tokens( source: &AuthoredSql<'_>, oracle: &Oracle, + keep_case: bool, ) -> Result { let AuthoredSql { sql, @@ -194,7 +195,7 @@ pub(crate) fn print_authored_tokens( } let guide = alignment[index].map(|at| &generated[at]); let start = output.len(); - match guide.filter(|guide| confirms_keyword(lexeme, guide)) { + match guide.filter(|guide| !keep_case && confirms_keyword(lexeme, guide)) { Some(guide) => { recases.insert(index, guide.raw.clone()); output.push_str(&guide.raw); diff --git a/crates/sqlquack-format/src/format/constants.rs b/crates/sqlquack-format/src/format/constants.rs index f074906c5..17c7c09d7 100644 --- a/crates/sqlquack-format/src/format/constants.rs +++ b/crates/sqlquack-format/src/format/constants.rs @@ -175,7 +175,7 @@ pub(crate) const SEPARABLE_KEYWORDS: [&str; 14] = [ "INTERSECT", ]; /// Binary operator nodes of the parse tree, with the spellings of their operator token. -pub(crate) const BINARY_OPERATOR_NODES: [(&str, &[&str]); 10] = [ +pub(crate) const BINARY_OPERATOR_NODES: [(&str, &[&str]); 13] = [ ("eq", &["="]), ("neq", &["!=", "<>"]), ("lt", &["<"]), @@ -185,6 +185,9 @@ pub(crate) const BINARY_OPERATOR_NODES: [(&str, &[&str]); 10] = [ ("add", &["+"]), ("sub", &["-"]), ("mul", &["*"]), + ("div", &["/"]), + ("mod", &["%"]), + ("int_div", &["//"]), ("concat", &["||"]), ]; /// The bytes the spacing guard reads as operator characters. @@ -197,8 +200,8 @@ pub(crate) const GUARD_OPERATORS: [&str; 22] = [ /// The operator of an assignment the tree keeps outside a binary operator node, such as `SET @v = 1`. pub(crate) const ASSIGNMENT_OPERATOR: &str = "="; /// Binary operators that whitespace may separate from a token they touch. -pub(crate) const SEPARABLE_OPERATORS: [&str; 12] = [ - "=", "<>", "!=", "<", ">", "<=", ">=", "+", "-", "*", "||", "->", +pub(crate) const SEPARABLE_OPERATORS: [&str; 15] = [ + "=", "<>", "!=", "<", ">", "<=", ">=", "+", "-", "*", "/", "//", "%", "||", "->", ]; /// Tokens a binary operator is never spaced from: name and path accessors. pub(crate) const UNSEPARABLE_NEIGHBOURS: [&str; 3] = [".", ":", "::"]; diff --git a/crates/sqlquack-format/src/format/models.rs b/crates/sqlquack-format/src/format/models.rs index ad80b6a24..c9d04c75f 100644 --- a/crates/sqlquack-format/src/format/models.rs +++ b/crates/sqlquack-format/src/format/models.rs @@ -26,6 +26,8 @@ pub struct FormatOptions { pub line_width: Option, /// Spaces per indent step. pub indent: usize, + /// Whether every token keeps the case it is written in; otherwise keywords and called names are upper-cased. + pub keep_case: bool, /// Printed widths of tokens that stand in for longer text, keyed by upper-case token text. pub token_widths: HashMap, /// Complexity guards for the validating parse. @@ -33,12 +35,13 @@ pub struct FormatOptions { } impl FormatOptions { - /// Default layout for `dialect`: no line width, two-space indent. + /// Default layout for `dialect`: no line width, two-space indent, keywords and called names upper-cased. pub fn new(dialect: FormatDialect) -> Self { Self { dialect, line_width: None, indent: DEFAULT_INDENT, + keep_case: false, token_widths: HashMap::new(), parse: ParseOptions::default(), } From 5f8f60cc95e2af4244bfe45332c73fd0aa55e343 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 15:49:40 +0000 Subject: [PATCH 3/8] feat(convert): lay out DuckDB output and helper macros with the formatter convert's text and JSON output, the Python API and the playground engine lay converted SQL and its helper macros out with sqlquack-format's DuckDB profile at 100 columns and a 4-space indent, keeping each token and its case. SQL the formatter refuses stays on one line; --compact still prints one line. The hand-written layout in sqlquack-blocks goes, and the docs and landing page show the new output. --- .github/workflows/build-distributions.yml | 6 +- Cargo.lock | 2 + crates/sqlquack-blocks/Cargo.toml | 1 + .../src/layout/_helpers/breaks.rs | 259 ------------------ .../src/layout/_helpers/mod.rs | 5 - .../src/layout/_helpers/plan.rs | 83 ------ .../src/layout/_helpers/tokens.rs | 142 ---------- .../sqlquack-blocks/src/layout/constants.rs | 51 +--- .../src/layout/main/checked_layout.rs | 36 +-- .../src/layout/main/layout_sql.rs | 10 +- crates/sqlquack-blocks/src/layout/mod.rs | 7 +- crates/sqlquack-blocks/src/layout/models.rs | 59 ---- crates/sqlquack-blocks/src/layout/tests.rs | 4 - .../src/layout/tests/test_layout_sql.rs | 110 -------- .../src/layout/tests/test_types.rs | 13 - crates/sqlquack/Cargo.toml | 1 + crates/sqlquack/src/analysis/models.rs | 3 +- crates/sqlquack/src/analysis/tests.rs | 2 + crates/sqlquack/src/analysis/tests/helpers.rs | 14 + .../src/analysis/tests/test_layout.rs | 126 +++++++++ .../sqlquack/src/analysis/tests/test_types.rs | 20 ++ .../src/cli/_helpers/convert_command.rs | 15 +- crates/sqlquack/tests/cli/helpers.rs | 8 +- crates/sqlquack/tests/cli/test_convert.rs | 6 +- website/public/index.html | 21 +- website/src/content/docs/docs/cli/convert.mdx | 7 +- .../src/content/docs/docs/helper-macros.mdx | 5 +- website/src/content/docs/docs/index.mdx | 5 +- website/src/content/docs/docs/policy.mdx | 5 +- website/src/content/docs/docs/project.mdx | 5 +- website/src/content/docs/docs/python.mdx | 2 +- website/src/content/docs/docs/quickstart.mdx | 5 +- website/src/content/docs/docs/sessions.mdx | 3 +- website/tests/playground.test.mjs | 31 ++- website/wasm/Cargo.lock | 11 + website/wasm/Cargo.toml | 1 + website/wasm/src/lib.rs | 11 +- 37 files changed, 298 insertions(+), 797 deletions(-) delete mode 100644 crates/sqlquack-blocks/src/layout/_helpers/breaks.rs delete mode 100644 crates/sqlquack-blocks/src/layout/_helpers/mod.rs delete mode 100644 crates/sqlquack-blocks/src/layout/_helpers/plan.rs delete mode 100644 crates/sqlquack-blocks/src/layout/_helpers/tokens.rs delete mode 100644 crates/sqlquack-blocks/src/layout/models.rs delete mode 100644 crates/sqlquack-blocks/src/layout/tests.rs delete mode 100644 crates/sqlquack-blocks/src/layout/tests/test_layout_sql.rs delete mode 100644 crates/sqlquack-blocks/src/layout/tests/test_types.rs create mode 100644 crates/sqlquack/src/analysis/tests/test_layout.rs diff --git a/.github/workflows/build-distributions.yml b/.github/workflows/build-distributions.yml index 3f78483c6..d548dd874 100644 --- a/.github/workflows/build-distributions.yml +++ b/.github/workflows/build-distributions.yml @@ -324,7 +324,7 @@ jobs: fi converted="$(echo "SELECT 1 AS total" | "${bin}/sqlquack" convert --no-config)" echo "${converted}" - grep -qx 'SELECT CAST(1 AS DECIMAL(1,0)) AS "TOTAL"' <<< "${converted}" + grep -qx 'SELECT CAST(1 AS DECIMAL(1, 0)) AS "TOTAL"' <<< "${converted}" # A file converted in a project folder, with no sqlquack.toml in it: the built-in policy. mkdir readme-smoke && cd readme-smoke printf "SELECT customer_id, SUM(total) AS revenue\nFROM orders\nGROUP BY customer_id\n" > query.sql @@ -333,7 +333,7 @@ jobs: "${bin}/sqlquack" check query.sql converted="$("${bin}/sqlquack" convert query.sql)" echo "${converted}" - grep -q 'CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE"' <<< "${converted}" + grep -q 'CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE"' <<< "${converted}" # run executes the conversion on the bundled DuckDB, over inline rows and over a CSV file. "${bin}/sqlquack" run query.sql --fixture 'orders=[[1, 2.5], [1, 4]]' --expect '[[1, 6.5]]' printf 'customer_id,total\n2,1.25\n' > orders.csv @@ -343,7 +343,7 @@ jobs: # The README's Python example. "${python[@]}" -c 'import sqlquack result = sqlquack.convert(sql="SELECT 1 AS total", options=sqlquack.ConvertOptions(no_config=True)) - assert result.sql == "SELECT CAST(1 AS DECIMAL(1,0)) AS \"TOTAL\"", result.sql' + assert result.sql == "SELECT CAST(1 AS DECIMAL(1, 0)) AS \"TOTAL\"", result.sql' - uses: actions/upload-artifact@v4 if: inputs.upload with: diff --git a/Cargo.lock b/Cargo.lock index ecbaa6013..6400dc393 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1370,6 +1370,7 @@ dependencies = [ "sqlquack-convert", "sqlquack-diagnostics", "sqlquack-evidence", + "sqlquack-format", "sqlquack-parse", "sqlquack-policy", "sqlquack-semantics", @@ -1387,6 +1388,7 @@ dependencies = [ "duckdb", "serde_json", "sqlquack-diagnostics", + "sqlquack-format", "sqlquack-semantics", ] diff --git a/crates/sqlquack-blocks/Cargo.toml b/crates/sqlquack-blocks/Cargo.toml index e1d90c1e1..b4a8a77cf 100644 --- a/crates/sqlquack-blocks/Cargo.toml +++ b/crates/sqlquack-blocks/Cargo.toml @@ -30,6 +30,7 @@ extensions = ["bundled", "duckdb/parquet"] duckdb = { workspace = true, optional = true } serde_json.workspace = true sqlquack-diagnostics.workspace = true +sqlquack-format.workspace = true sqlquack-semantics = { workspace = true, features = ["snowflake"] } [dev-dependencies] diff --git a/crates/sqlquack-blocks/src/layout/_helpers/breaks.rs b/crates/sqlquack-blocks/src/layout/_helpers/breaks.rs deleted file mode 100644 index 4c73c7b2a..000000000 --- a/crates/sqlquack-blocks/src/layout/_helpers/breaks.rs +++ /dev/null @@ -1,259 +0,0 @@ -//! Where lines break: before a token, and after one that asks for a break. - -use crate::layout::_helpers::plan::Plan; -use crate::layout::_helpers::tokens::{Token, TokenKind, tokens}; -use crate::layout::constants::{ - BY_CLAUSE_WORDS, CLAUSE_WORDS, INDENT, INLINE_WIDTH, JOIN_MODIFIERS, NON_CALL_WORDS, - SELECT_MODIFIERS, SET_OPERATION_MODIFIERS, SET_OPERATIONS, -}; -use crate::layout::models::{Case, Clause, Frame, Walk}; - -/// `sql` one clause and select item per line, nested queries and long `CASE`s indented by depth. -pub(crate) fn lay_out(sql: &str) -> String { - let tokens: Vec> = tokens(sql); - let plan = Plan::new(&tokens); - let mut walk = start(plan.width(0, tokens.len().saturating_sub(1)) > INLINE_WIDTH); - for index in 0..tokens.len() { - let level = walk.line_break(&tokens, index); - walk.push_token(&tokens, index, level); - walk.forced = walk.after_token(&tokens, index, &plan); - } - walk.laid_out -} - -/// The walk over a statement, which breaks its lines when `broken`. -fn start(broken: bool) -> Walk { - Walk { - frames: vec![Frame { - query: true, - broken, - inline: !broken, - call: false, - base: 0, - close: 0, - clause: Clause::Other, - item_pending: false, - }], - cases: Vec::new(), - line_indent: 0, - forced: None, - laid_out: String::new(), - } -} - -impl Walk { - /// Appends `tokens[index]`, on a new line at `level` or after its own spacing. - pub(crate) fn push_token(&mut self, tokens: &[Token<'_>], index: usize, level: Option) { - let token = &tokens[index]; - match (index, level) { - (0, _) => {} - (_, Some(level)) => { - self.laid_out.push('\n'); - self.laid_out.push_str(&INDENT.repeat(level)); - self.line_indent = level; - } - (_, None) if token.spaced => self.laid_out.push(' '), - (_, None) => {} - } - self.laid_out.push_str(token.text); - } - - /// The indentation level of the new line `tokens[index]` starts, or `None` to keep its line. - pub(crate) fn line_break(&mut self, tokens: &[Token<'_>], index: usize) -> Option { - if let Some((_, level)) = self.forced.filter(|(at, _)| *at == index) { - return Some(level); - } - let token = &tokens[index]; - let depth = self.frames.len(); - let case = self.cases.last().filter(|case| case.depth == depth); - let frame = self.frames.last_mut()?; - if token.is_symbol(")") { - return (depth > 1 && frame.query && frame.broken).then_some(frame.close); - } - if frame.inline { - return None; - } - if let Some(level) = case.and_then(|case| case_break(token, case)) { - return Some(level); - } - if !frame.query || !frame.broken { - return None; - } - if frame.item_pending { - return frame.first_item_break(tokens, index); - } - (starts_clause(tokens, index) || is_set_operation(tokens, index)).then_some(frame.base) - } - - /// Opens or closes what `tokens[index]` opens or closes, and the break it asks for after it. - pub(crate) fn after_token( - &mut self, - tokens: &[Token<'_>], - index: usize, - plan: &Plan, - ) -> Option<(usize, usize)> { - let token = &tokens[index]; - match token { - token if token.is_symbol("(") => self.open_frame(tokens, index, plan), - token if token.is_symbol(")") => { - if self.frames.len() > 1 { - self.frames.pop(); - } - None - } - token if token.is("CASE") => { - let frame = self.frames.last()?; - let broken = !frame.inline - && !frame.call - && plan.width(index, plan.case_end(index)) > INLINE_WIDTH; - self.cases.push(Case { - broken, - indent: self.line_indent, - depth: self.frames.len(), - }); - None - } - token if token.is("END") => { - self.cases.pop(); - None - } - token if token.kind == TokenKind::LineComment => Some((index + 1, self.line_indent)), - _ => self.frames.last_mut()?.clause_break(tokens, index), - } - } - - /// Opens the frame of the parenthesis at `tokens[index]`, breaking inside a long query. - fn open_frame( - &mut self, - tokens: &[Token<'_>], - index: usize, - plan: &Plan, - ) -> Option<(usize, usize)> { - let frame = self.frames.last()?; - let query = plan.opens_query(tokens, index); - let broken = query && !frame.inline && plan.width(index, plan.close(index)) > INLINE_WIDTH; - let base = self.line_indent + 1; - let opened = Frame { - query, - broken, - inline: frame.inline || (query && !broken), - call: !query && (frame.call || calls(tokens, index)), - base, - close: self.line_indent, - clause: Clause::Other, - item_pending: false, - }; - self.frames.push(opened); - broken.then_some((index + 1, base)) - } -} - -impl Frame { - /// The line the select list's first item starts, after any `DISTINCT` or `DISTINCT ON (…)`. - fn first_item_break(&mut self, tokens: &[Token<'_>], index: usize) -> Option { - let token = &tokens[index]; - let modifier = SELECT_MODIFIERS.iter().any(|word| token.is(word)) - || (token.is_symbol("(") && tokens[index - 1].is("ON")); - if modifier { - return None; - } - self.item_pending = false; - Some(self.base + 1) - } - - /// The clause `tokens[index]` moves a broken query into, and the break its commas ask for. - fn clause_break(&mut self, tokens: &[Token<'_>], index: usize) -> Option<(usize, usize)> { - let token = &tokens[index]; - if !self.query || !self.broken { - return None; - } - match token { - token if token.is("SELECT") => { - (self.clause, self.item_pending) = (Clause::Select, true); - None - } - token if token.is("WITH") && self.clause == Clause::Other => { - self.clause = Clause::With; - None - } - token if token.is_symbol(",") => match self.clause { - Clause::Select => Some((index + 1, self.base + 1)), - Clause::With => Some((index + 1, self.base)), - Clause::Other => None, - }, - _ if is_set_operation(tokens, index) => { - self.clause = Clause::Other; - Some((after_modifiers(tokens, index), self.base)) - } - _ if starts_clause(tokens, index) => { - self.clause = Clause::Other; - None - } - _ => None, - } - } -} - -/// The level of the line `token` starts within `case`, when the `CASE` breaks. -fn case_break(token: &Token<'_>, case: &Case) -> Option { - match case.broken { - true if token.is("WHEN") || token.is("ELSE") => Some(case.indent + 1), - true if token.is("END") => Some(case.indent), - _ => None, - } -} - -/// The index of the first token after the set operation at `index` and its modifiers. -fn after_modifiers(tokens: &[Token<'_>], index: usize) -> usize { - (index + 1..tokens.len()) - .find(|at| !is_set_operation_modifier(&tokens[*at])) - .unwrap_or(tokens.len()) -} - -/// Whether `token` is `ALL`, `DISTINCT`, `BY` or `NAME` after a set operation. -fn is_set_operation_modifier(token: &Token<'_>) -> bool { - SET_OPERATION_MODIFIERS.iter().any(|word| token.is(word)) -} - -/// Whether `token` can come before `JOIN` in a join. -fn is_join_modifier(token: &Token<'_>) -> bool { - JOIN_MODIFIERS.iter().any(|word| token.is(word)) -} - -/// Whether `tokens[index]` starts a clause: a clause word, `GROUP BY`, `ORDER BY` or a join. -fn starts_clause(tokens: &[Token<'_>], index: usize) -> bool { - let token = &tokens[index]; - let previous = index.checked_sub(1).map(|at| &tokens[at]); - let next = tokens.get(index + 1); - if token.is("FROM") && previous.is_some_and(|previous| previous.is("DISTINCT")) { - return false; - } - let joins = (token.is("JOIN") || is_join_modifier(token)) - && !previous.is_some_and(is_join_modifier) - && tokens[index..] - .iter() - .find(|token| !is_join_modifier(token)) - .is_some_and(|token| token.is("JOIN")); - CLAUSE_WORDS.iter().any(|word| token.is(word)) - || (BY_CLAUSE_WORDS.iter().any(|word| token.is(word)) - && next.is_some_and(|next| next.is("BY"))) - || joins -} - -/// Whether `tokens[index]` is a set operation, not the `EXCEPT` of a `* EXCEPT (…)` column list. -fn is_set_operation(tokens: &[Token<'_>], index: usize) -> bool { - SET_OPERATIONS.iter().any(|word| tokens[index].is(word)) - && !(tokens[index].is("EXCEPT") && index > 0 && tokens[index - 1].is_symbol("*")) -} - -/// Whether the parenthesis at `tokens[index]` calls a function rather than grouping. -fn calls(tokens: &[Token<'_>], index: usize) -> bool { - let Some(previous) = index.checked_sub(1).map(|at| &tokens[at]) else { - return false; - }; - match previous.kind { - TokenKind::Word => !NON_CALL_WORDS.iter().any(|word| previous.is(word)), - TokenKind::Quoted => true, - _ => previous.is_symbol("]") || previous.is_symbol(")"), - } -} diff --git a/crates/sqlquack-blocks/src/layout/_helpers/mod.rs b/crates/sqlquack-blocks/src/layout/_helpers/mod.rs deleted file mode 100644 index ad51c813c..000000000 --- a/crates/sqlquack-blocks/src/layout/_helpers/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -//! Tokens, the facts about them, and where lines break. - -pub(crate) mod breaks; -pub(crate) mod plan; -pub(crate) mod tokens; diff --git a/crates/sqlquack-blocks/src/layout/_helpers/plan.rs b/crates/sqlquack-blocks/src/layout/_helpers/plan.rs deleted file mode 100644 index 4ae876b27..000000000 --- a/crates/sqlquack-blocks/src/layout/_helpers/plan.rs +++ /dev/null @@ -1,83 +0,0 @@ -//! What the layout knows before it starts: matching parentheses and `END`s, and widths. - -use crate::layout::_helpers::tokens::{Token, TokenKind}; -use crate::layout::constants::{QUERY_OPENERS, SET_OPERATIONS}; - -/// Facts about the tokens, each found in one pass over them. -pub(crate) struct Plan { - /// For each opening parenthesis, the index of its closing one. - closes: Vec, - /// For each `CASE`, the index of its `END`. - ends: Vec, - /// For each opening parenthesis, whether a set operation stands directly inside it. - set_operations: Vec, - /// Characters other than whitespace before each token, and in all of them at the end. - widths: Vec, -} - -impl Plan { - /// The facts about `tokens`; an unmatched parenthesis or `CASE` reaches the last token. - pub(crate) fn new(tokens: &[Token<'_>]) -> Self { - let last = tokens.len().saturating_sub(1); - let (mut closes, mut ends) = (vec![last; tokens.len()], vec![last; tokens.len()]); - let mut set_operations = vec![false; tokens.len()]; - let (mut parentheses, mut cases): (Vec, Vec) = (Vec::new(), Vec::new()); - let mut widths = Vec::with_capacity(tokens.len() + 1); - widths.push(0); - for (index, token) in tokens.iter().enumerate() { - widths.push(widths[index] + token.text.chars().count()); - match token { - token if token.is_symbol("(") => parentheses.push(index), - token if token.is_symbol(")") => { - if let Some(open) = parentheses.pop() { - closes[open] = index; - } - } - token if token.is("CASE") => cases.push(index), - token if token.is("END") => { - if let Some(open) = cases.pop() { - ends[open] = index; - } - } - token if SET_OPERATIONS.iter().any(|word| token.is(word)) => { - if let Some(open) = parentheses.last() { - set_operations[*open] = true; - } - } - _ => {} - } - } - Self { - closes, - ends, - set_operations, - widths, - } - } - - /// The index of the parenthesis closing the one at `open`. - pub(crate) fn close(&self, open: usize) -> usize { - self.closes[open] - } - - /// The index of the `END` closing the `CASE` at `open`. - pub(crate) fn case_end(&self, open: usize) -> usize { - self.ends[open] - } - - /// Characters other than whitespace from token `first` through token `last`. - pub(crate) fn width(&self, first: usize, last: usize) -> usize { - self.widths - .get(last + 1) - .map_or(0, |end| end.saturating_sub(self.widths[first])) - } - - /// Whether the parenthesis at `open` holds a query or a set operation of queries. - pub(crate) fn opens_query(&self, tokens: &[Token<'_>], open: usize) -> bool { - let first = tokens[open + 1..] - .iter() - .find(|token| !matches!(token.kind, TokenKind::LineComment | TokenKind::BlockComment)); - self.set_operations[open] - || first.is_some_and(|token| QUERY_OPENERS.iter().any(|word| token.is(word))) - } -} diff --git a/crates/sqlquack-blocks/src/layout/_helpers/tokens.rs b/crates/sqlquack-blocks/src/layout/_helpers/tokens.rs deleted file mode 100644 index 59f75fc9c..000000000 --- a/crates/sqlquack-blocks/src/layout/_helpers/tokens.rs +++ /dev/null @@ -1,142 +0,0 @@ -//! The tokens of DuckDB SQL, for a layout that changes only the whitespace between them. - -/// What a token is, as far as the layout needs to know. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum TokenKind { - /// A bare word: a keyword, a function name or an unquoted name or number. - Word, - /// A string literal or a quoted name, kept whole. - Quoted, - /// A `--` comment, which ends its line. - LineComment, - /// A `/* */` comment. - BlockComment, - /// One character of punctuation or an operator. - Symbol, -} - -/// One token of the SQL and whether whitespace came before it. -#[derive(Clone, Copy, Debug)] -pub(crate) struct Token<'sql> { - /// The token's text, exactly as written. - pub(crate) text: &'sql str, - /// What kind of token it is. - pub(crate) kind: TokenKind, - /// Whether whitespace separated it from the token before. - pub(crate) spaced: bool, -} - -impl Token<'_> { - /// Whether the token is the bare word `word`, in any case. - pub(crate) fn is(&self, word: &str) -> bool { - self.kind == TokenKind::Word && self.text.eq_ignore_ascii_case(word) - } - - /// Whether the token is the symbol `symbol`. - pub(crate) fn is_symbol(&self, symbol: &str) -> bool { - self.kind == TokenKind::Symbol && self.text == symbol - } -} - -/// The tokens of `sql`; strings, quoted names and comments stay single tokens. -pub(crate) fn tokens(sql: &str) -> Vec> { - let bytes = sql.as_bytes(); - let mut tokens: Vec> = Vec::new(); - let (mut index, mut spaced) = (0, false); - while let Some(&byte) = bytes.get(index) { - if byte.is_ascii_whitespace() { - spaced = true; - index += 1; - continue; - } - let (end, kind) = match (byte, bytes.get(index + 1).copied()) { - (b'\'' | b'"', _) => (quoted_end(bytes, index), TokenKind::Quoted), - (b'-', Some(b'-')) => (line_end(bytes, index), TokenKind::LineComment), - (b'/', Some(b'*')) => (comment_end(bytes, index), TokenKind::BlockComment), - (byte, _) if is_word_byte(byte) => (word_end(bytes, index), TokenKind::Word), - _ => (symbol_end(sql, index), TokenKind::Symbol), - }; - tokens.push(Token { - text: &sql[index..end], - kind, - spaced, - }); - (index, spaced) = (end, false); - } - tokens -} - -/// Text whose literals or comments the tokens may misread: `$`, `E''`, unclosed or nested ones. -pub(crate) fn unmodelled(tokens: &[Token<'_>]) -> Option<&'static str> { - tokens - .iter() - .enumerate() - .find_map(|(index, token)| match token.kind { - TokenKind::Symbol if token.is_symbol("$") => Some("a `$`"), - TokenKind::Quoted - if index > 0 && !token.spaced && tokens[index - 1].kind == TokenKind::Word => - { - Some("a prefixed string") - } - TokenKind::Quoted if !token.text[1..].ends_with(&token.text[..1]) => { - Some("an unclosed quote") - } - TokenKind::BlockComment - if !token.text.ends_with("*/") || token.text[2..].contains("/*") => - { - Some("an unclosed or nested comment") - } - _ => None, - }) -} - -/// Whether `byte` continues a bare word; bytes of non-ASCII characters count as word bytes. -fn is_word_byte(byte: u8) -> bool { - byte.is_ascii_alphanumeric() || byte == b'_' || !byte.is_ascii() -} - -/// The index past the quoted text opening at `start`, where a doubled quote is part of the text. -fn quoted_end(bytes: &[u8], start: usize) -> usize { - let quote = bytes[start]; - let mut index = start + 1; - while let Some(&byte) = bytes.get(index) { - match (byte == quote, bytes.get(index + 1) == Some("e)) { - (true, true) => index += 2, - (true, false) => return index + 1, - (false, _) => index += 1, - } - } - index -} - -/// The index of the line break ending the `--` comment at `start`, or the end of the SQL. -fn line_end(bytes: &[u8], start: usize) -> usize { - bytes[start..] - .iter() - .position(|byte| *byte == b'\n') - .map_or(bytes.len(), |offset| start + offset) -} - -/// The index past the `*/` closing the comment at `start`, or the end of the SQL. -fn comment_end(bytes: &[u8], start: usize) -> usize { - bytes[start + 2..] - .windows(2) - .position(|pair| pair == b"*/") - .map_or(bytes.len(), |offset| start + 2 + offset + 2) -} - -/// The index past the bare word at `start`. -fn word_end(bytes: &[u8], start: usize) -> usize { - bytes[start..] - .iter() - .position(|byte| !is_word_byte(*byte)) - .map_or(bytes.len(), |offset| start + offset) -} - -/// The index past the one character at `start`. -fn symbol_end(sql: &str, start: usize) -> usize { - sql[start..] - .chars() - .next() - .map_or(sql.len(), |character| start + character.len_utf8()) -} diff --git a/crates/sqlquack-blocks/src/layout/constants.rs b/crates/sqlquack-blocks/src/layout/constants.rs index 110d453a2..3b66105c2 100644 --- a/crates/sqlquack-blocks/src/layout/constants.rs +++ b/crates/sqlquack-blocks/src/layout/constants.rs @@ -1,48 +1,7 @@ -//! The layout's indentation, width and clause words. +//! The layout's line width and indentation. -/// One level of indentation. -pub const INDENT: &str = " "; +/// Lines longer than this many characters are broken where the formatter can break them. +pub const LINE_WIDTH: usize = 100; -/// The longest query or `CASE` kept on one line, counted in characters other than whitespace. -pub const INLINE_WIDTH: usize = 60; - -/// Words that start a clause of a query on a new line. -pub const CLAUSE_WORDS: [&str; 8] = [ - "SELECT", "FROM", "WHERE", "HAVING", "QUALIFY", "WINDOW", "LIMIT", "OFFSET", -]; - -/// Words that open a query inside parentheses, besides a set operation at its level. -pub const QUERY_OPENERS: [&str; 3] = ["SELECT", "WITH", "VALUES"]; - -/// Words that start a clause when `BY` follows them. -pub const BY_CLAUSE_WORDS: [&str; 2] = ["GROUP", "ORDER"]; - -/// Set operations, which stand on a line of their own. -pub const SET_OPERATIONS: [&str; 3] = ["UNION", "INTERSECT", "EXCEPT"]; - -/// Words that can come between a set operation and the query after it. -pub const SET_OPERATION_MODIFIERS: [&str; 4] = ["ALL", "DISTINCT", "BY", "NAME"]; - -/// Words that can come before `JOIN` in a join. -pub const JOIN_MODIFIERS: [&str; 11] = [ - "LEFT", - "RIGHT", - "FULL", - "INNER", - "OUTER", - "CROSS", - "NATURAL", - "ASOF", - "POSITIONAL", - "SEMI", - "ANTI", -]; - -/// Words after which a parenthesis groups an expression rather than calling a function. -pub const NON_CALL_WORDS: [&str; 15] = [ - "SELECT", "WHERE", "AND", "OR", "NOT", "WHEN", "THEN", "ELSE", "IN", "ON", "AS", "HAVING", - "QUALIFY", "BY", "IS", -]; - -/// Words that can come between `SELECT` and its first item. -pub const SELECT_MODIFIERS: [&str; 3] = ["DISTINCT", "ALL", "ON"]; +/// Spaces per level of indentation. +pub const INDENT: usize = 4; diff --git a/crates/sqlquack-blocks/src/layout/main/checked_layout.rs b/crates/sqlquack-blocks/src/layout/main/checked_layout.rs index 5640eb3ac..43307ef93 100644 --- a/crates/sqlquack-blocks/src/layout/main/checked_layout.rs +++ b/crates/sqlquack-blocks/src/layout/main/checked_layout.rs @@ -1,31 +1,23 @@ -//! `checked_layout`: the layout of DuckDB SQL, checked to change only whitespace. +//! `checked_layout`: the layout of converted SQL, checked to change only whitespace. -use crate::layout::_helpers::breaks::lay_out; -use crate::layout::_helpers::tokens::{tokens, unmodelled}; +use sqlquack_format::{FormatDialect, FormatOptions, format_sql}; -/// `sql` laid out, or why its text is unmodelled or its layout changes more than whitespace. -pub fn checked_layout(sql: &str) -> Result { - if let Some(reason) = unmodelled(&tokens(sql)) { - return Err(format!( - "it holds {reason}, which the layout does not model" - )); - } - let laid_out = lay_out(sql); - let texts = |sql: &str| -> Vec { - tokens(sql) - .iter() - .map(|token| token.text.to_owned()) - .collect() +use crate::layout::constants::{INDENT, LINE_WIDTH}; + +/// `sql` in `dialect` laid out with its tokens and their case kept, or why it was refused. +pub fn checked_layout(sql: &str, dialect: FormatDialect) -> Result { + let options = FormatOptions { + line_width: Some(LINE_WIDTH), + indent: INDENT, + keep_case: true, + ..FormatOptions::new(dialect) }; - if texts(sql) != texts(&laid_out) { - return Err("its tokens differ from the single-line SQL's".to_owned()); - } + let laid_out = format_sql(sql, &options) + .map_err(|refusal| format!("the formatter refused it: {}", refusal.reason))? + .sql; let stripped = |sql: &str| -> String { sql.chars().filter(|c| !c.is_whitespace()).collect() }; if stripped(sql) != stripped(&laid_out) { return Err("its characters other than whitespace differ".to_owned()); } - if lay_out(&laid_out) != laid_out { - return Err("laying it out again changes it".to_owned()); - } Ok(laid_out) } diff --git a/crates/sqlquack-blocks/src/layout/main/layout_sql.rs b/crates/sqlquack-blocks/src/layout/main/layout_sql.rs index 172852186..b1d1da030 100644 --- a/crates/sqlquack-blocks/src/layout/main/layout_sql.rs +++ b/crates/sqlquack-blocks/src/layout/main/layout_sql.rs @@ -1,8 +1,10 @@ -//! `layout_sql`: DuckDB SQL laid out over lines, or unchanged where the layout cannot be checked. +//! `layout_sql`: converted SQL laid out over lines, or unchanged where the layout is refused. + +use sqlquack_format::FormatDialect; use crate::layout::main::checked_layout::checked_layout; -/// `sql` laid out by [`checked_layout`], or `sql` itself on one line when the check fails. -pub fn layout_sql(sql: &str) -> String { - checked_layout(sql).unwrap_or_else(|_| sql.to_owned()) +/// `sql` laid out by [`checked_layout`], or `sql` itself on one line when it is refused. +pub fn layout_sql(sql: &str, dialect: FormatDialect) -> String { + checked_layout(sql, dialect).unwrap_or_else(|_| sql.to_owned()) } diff --git a/crates/sqlquack-blocks/src/layout/mod.rs b/crates/sqlquack-blocks/src/layout/mod.rs index 062f5bfcc..c0352e2b6 100644 --- a/crates/sqlquack-blocks/src/layout/mod.rs +++ b/crates/sqlquack-blocks/src/layout/mod.rs @@ -1,9 +1,4 @@ -//! A simple layout of DuckDB SQL over lines, changing only the whitespace between tokens. +//! Converted SQL laid out over lines by the formatter, changing only whitespace. -pub(crate) mod _helpers; pub mod constants; pub mod main; -pub(crate) mod models; - -#[cfg(test)] -mod tests; diff --git a/crates/sqlquack-blocks/src/layout/models.rs b/crates/sqlquack-blocks/src/layout/models.rs deleted file mode 100644 index 69e1e00b9..000000000 --- a/crates/sqlquack-blocks/src/layout/models.rs +++ /dev/null @@ -1,59 +0,0 @@ -//! The state the layout keeps while it walks the tokens. - -/// The clause a query is in, which decides where its commas break. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum Clause { - /// The select list: one item per line. - Select, - /// The common table expressions: one per line. - With, - /// Any other clause: commas stay inline. - Other, -} - -/// One pair of parentheses being laid out, or the statement itself. -#[derive(Debug)] -pub(crate) struct Frame { - /// Whether it holds a query rather than an expression. - pub(crate) query: bool, - /// Whether its lines break; a short query stays on one line. - pub(crate) broken: bool, - /// Whether nothing inside it may break, because an enclosing query stays on one line. - pub(crate) inline: bool, - /// Whether a function call encloses it within its query, so a `CASE` in it stays inline. - pub(crate) call: bool, - /// The indentation level of its clauses. - pub(crate) base: usize, - /// The indentation level of its closing parenthesis. - pub(crate) close: usize, - /// The clause it is in. - pub(crate) clause: Clause, - /// Whether the select list's first item is still to come. - pub(crate) item_pending: bool, -} - -/// One `CASE` being laid out. -#[derive(Debug)] -pub(crate) struct Case { - /// Whether it breaks at `WHEN`, `ELSE` and `END`. - pub(crate) broken: bool, - /// The indentation level of the line it starts on. - pub(crate) indent: usize, - /// How many frames were open where it starts. - pub(crate) depth: usize, -} - -/// Where the layout is: open frames and `CASE`s, the line's indentation and a pending break. -#[derive(Debug)] -pub(crate) struct Walk { - /// The open parentheses, outermost first, the statement's own frame at the bottom. - pub(crate) frames: Vec, - /// The open `CASE`s, outermost first. - pub(crate) cases: Vec, - /// The indentation level of the current line. - pub(crate) line_indent: usize, - /// A line break the previous token asked for: the token it comes before, and its level. - pub(crate) forced: Option<(usize, usize)>, - /// The SQL laid out so far. - pub(crate) laid_out: String, -} diff --git a/crates/sqlquack-blocks/src/layout/tests.rs b/crates/sqlquack-blocks/src/layout/tests.rs deleted file mode 100644 index e08edccf2..000000000 --- a/crates/sqlquack-blocks/src/layout/tests.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[path = "tests/test_layout_sql.rs"] -mod test_layout_sql; -#[path = "tests/test_types.rs"] -mod test_types; diff --git a/crates/sqlquack-blocks/src/layout/tests/test_layout_sql.rs b/crates/sqlquack-blocks/src/layout/tests/test_layout_sql.rs deleted file mode 100644 index 05ccce63b..000000000 --- a/crates/sqlquack-blocks/src/layout/tests/test_layout_sql.rs +++ /dev/null @@ -1,110 +0,0 @@ -use crate::layout::main::checked_layout::checked_layout; -use crate::layout::main::layout_sql::layout_sql; -use crate::layout::tests::test_types::{FallbackTestCase, LayoutTestCase}; - -#[test] -fn given_converted_sql_when_laid_out_then_clauses_items_and_long_cases_break_by_depth() { - let test_cases = [ - LayoutTestCase { - description: "a short query stays on one line", - sql: r#"SELECT CAST(1 AS DECIMAL(1,0)) AS "1""#, - expected_layout: r#"SELECT CAST(1 AS DECIMAL(1,0)) AS "1""#, - }, - LayoutTestCase { - description: "a common table expression, a join, a long CASE and a short subquery", - sql: r#"WITH "X" AS (SELECT "O"."CUST" AS "CUST", sum("O"."AMT") AS "T" FROM "ORDERS" AS "O" GROUP BY "O"."CUST" HAVING (sum("O"."AMT") > 10)) SELECT "C"."NAME" AS "NAME", (CASE WHEN ("X"."T" > 100) THEN 'big' WHEN ("X"."T" > 50) THEN 'mid' ELSE 'small' END) AS "SZ" FROM "X" AS "X" LEFT JOIN "CUSTOMERS" AS "C" ON ("C"."NAME" = "X"."CUST") WHERE ("X"."T" IN (SELECT "O"."AMT" FROM "ORDERS" AS "O")) ORDER BY "X"."T" DESC NULLS FIRST LIMIT 5"#, - expected_layout: r#"WITH "X" AS ( - SELECT - "O"."CUST" AS "CUST", - sum("O"."AMT") AS "T" - FROM "ORDERS" AS "O" - GROUP BY "O"."CUST" - HAVING (sum("O"."AMT") > 10) -) -SELECT - "C"."NAME" AS "NAME", - (CASE - WHEN ("X"."T" > 100) THEN 'big' - WHEN ("X"."T" > 50) THEN 'mid' - ELSE 'small' - END) AS "SZ" -FROM "X" AS "X" -LEFT JOIN "CUSTOMERS" AS "C" ON ("C"."NAME" = "X"."CUST") -WHERE ("X"."T" IN (SELECT "O"."AMT" FROM "ORDERS" AS "O")) -ORDER BY "X"."T" DESC NULLS FIRST -LIMIT 5"#, - }, - LayoutTestCase { - description: "a set operation stands on its own line, and a short branch stays inline", - sql: r#"(SELECT "T"."ID" AS "ID", "T"."NAME" AS "NAME" FROM "T" AS "T" WHERE ("T"."ID" > 1)) UNION ALL (SELECT "U"."ID" AS "ID", "U"."NAME" AS "NAME" FROM "U" AS "U")"#, - expected_layout: r#"( - SELECT - "T"."ID" AS "ID", - "T"."NAME" AS "NAME" - FROM "T" AS "T" - WHERE ("T"."ID" > 1) -) -UNION ALL (SELECT "U"."ID" AS "ID", "U"."NAME" AS "NAME" FROM "U" AS "U")"#, - }, - LayoutTestCase { - description: "a long CASE inside a function call stays inline", - sql: r#"SELECT list_transform([1], lambda x: CASE WHEN abs(x) > 9007199254740992 THEN CAST('a long raise message' AS INTEGER) ELSE x END)[1] AS "E" FROM "T" AS "T""#, - expected_layout: r#"SELECT - list_transform([1], lambda x: CASE WHEN abs(x) > 9007199254740992 THEN CAST('a long raise message' AS INTEGER) ELSE x END)[1] AS "E" -FROM "T" AS "T""#, - }, - LayoutTestCase { - description: "keywords in strings, quoted names, IS DISTINCT FROM and * EXCEPT do not break", - sql: r#"SELECT DISTINCT * EXCEPT ("B"), 'x FROM y, z' AS "WHERE, FROM" FROM "T" AS "T" WHERE ("T"."A" IS DISTINCT FROM "T"."B")"#, - expected_layout: r#"SELECT DISTINCT - * EXCEPT ("B"), - 'x FROM y, z' AS "WHERE, FROM" -FROM "T" AS "T" -WHERE ("T"."A" IS DISTINCT FROM "T"."B")"#, - }, - ]; - for test_case in test_cases { - let laid_out = checked_layout(test_case.sql); - assert_eq!( - laid_out.as_deref(), - Ok(test_case.expected_layout), - "{}", - test_case.description - ); - } -} - -#[test] -fn given_text_the_layout_does_not_model_when_laid_out_then_the_single_line_is_kept() { - let test_cases = [ - FallbackTestCase { - description: "a dollar-quoted string holding clause words and doubled spaces", - sql: r#"SELECT $$a FROM b, WHERE c$$ AS "TEXT", "T"."A" AS "A" FROM "T" AS "T" WHERE ("T"."A" > 1)"#, - expected_reason: "a `$`", - }, - FallbackTestCase { - description: "an escape string with an escaped quote and doubled spaces", - sql: r#"SELECT E'it\'s FROM here' AS "TEXT", "T"."A" AS "A" FROM "T" AS "T" WHERE ("T"."A" > 1)"#, - expected_reason: "a prefixed string", - }, - FallbackTestCase { - description: "a nested comment holding a clause word", - sql: r#"SELECT /* a /* b */ FROM c */ "T"."A" AS "A", "T"."B" AS "B" FROM "T" AS "T" WHERE ("T"."A" > 1)"#, - expected_reason: "an unclosed or nested comment", - }, - ]; - for test_case in test_cases { - let refusal = checked_layout(test_case.sql).expect_err(test_case.description); - assert!( - refusal.contains(test_case.expected_reason), - "{}: {refusal}", - test_case.description - ); - assert_eq!( - layout_sql(test_case.sql), - test_case.sql, - "{}", - test_case.description - ); - } -} diff --git a/crates/sqlquack-blocks/src/layout/tests/test_types.rs b/crates/sqlquack-blocks/src/layout/tests/test_types.rs deleted file mode 100644 index 74f19ee99..000000000 --- a/crates/sqlquack-blocks/src/layout/tests/test_types.rs +++ /dev/null @@ -1,13 +0,0 @@ -/// Single-line SQL and its expected layout. -pub(crate) struct LayoutTestCase { - pub(crate) description: &'static str, - pub(crate) sql: &'static str, - pub(crate) expected_layout: &'static str, -} - -/// SQL holding text the layout does not model, and the reason its check gives. -pub(crate) struct FallbackTestCase { - pub(crate) description: &'static str, - pub(crate) sql: &'static str, - pub(crate) expected_reason: &'static str, -} diff --git a/crates/sqlquack/Cargo.toml b/crates/sqlquack/Cargo.toml index a5f70e7c6..b3af40adf 100644 --- a/crates/sqlquack/Cargo.toml +++ b/crates/sqlquack/Cargo.toml @@ -35,6 +35,7 @@ sqlquack-convert = { workspace = true, features = ["engine", "snowflake"] } sqlquack-diagnostics = { workspace = true, features = ["snowflake"] } sqlquack-parse.workspace = true sqlquack-blocks = { workspace = true, features = ["engine"] } +sqlquack-format = { workspace = true, features = ["duckdb"] } sqlquack-evidence.workspace = true sqlquack-policy.workspace = true sqlquack-semantics = { workspace = true, features = ["snowflake"] } diff --git a/crates/sqlquack/src/analysis/models.rs b/crates/sqlquack/src/analysis/models.rs index 374a815e6..9a98f584b 100644 --- a/crates/sqlquack/src/analysis/models.rs +++ b/crates/sqlquack/src/analysis/models.rs @@ -17,6 +17,7 @@ use sqlquack_evidence::constants::{INPUT_RELATION, ORDINAL_COLUMN}; use sqlquack_evidence::replay::models::{PythonFunction, PythonReplay}; use sqlquack_evidence::verify::models::Conversion; use sqlquack_evidence::verify::types::CaseConverter; +use sqlquack_format::FormatDialect; use sqlquack_policy::policy::constants::QUOTED_IDENTIFIERS_IGNORE_CASE; use sqlquack_policy::policy::models::SessionValue; use sqlquack_semantics::snowflake::typing::types::Schema; @@ -154,7 +155,7 @@ impl CaseConverter for PipelineConverter { } fn laid_out(&self, sql: &str) -> Result { - checked_layout(sql) + checked_layout(sql, FormatDialect::DuckDB) } } diff --git a/crates/sqlquack/src/analysis/tests.rs b/crates/sqlquack/src/analysis/tests.rs index 87483bd5d..765317b89 100644 --- a/crates/sqlquack/src/analysis/tests.rs +++ b/crates/sqlquack/src/analysis/tests.rs @@ -32,6 +32,8 @@ mod test_float_text; mod test_guarded_conditions; #[path = "tests/test_held_variants.rs"] mod test_held_variants; +#[path = "tests/test_layout.rs"] +mod test_layout; #[path = "tests/test_load_override.rs"] mod test_load_override; #[path = "tests/test_locations.rs"] diff --git a/crates/sqlquack/src/analysis/tests/helpers.rs b/crates/sqlquack/src/analysis/tests/helpers.rs index 135bed1b1..cd1f521e1 100644 --- a/crates/sqlquack/src/analysis/tests/helpers.rs +++ b/crates/sqlquack/src/analysis/tests/helpers.rs @@ -1078,3 +1078,17 @@ const TRANSLATED: &str = "translated"; /// The diagnostic code of SQL that does not parse. const PARSE_FAILURE_CODE: &str = "SQ0001"; + +/// Why DuckDB did not create `statement` laid out as convert prints it, if it did not. +pub(crate) fn laid_out_creation_failure( + connection: &duckdb::Connection, + statement: &str, +) -> Result<(), String> { + let laid_out = sqlquack_blocks::layout::main::checked_layout::checked_layout( + statement, + sqlquack_format::FormatDialect::DuckDB, + )?; + connection + .execute_batch(&laid_out) + .map_err(|error| format!("{laid_out}: {error}")) +} diff --git a/crates/sqlquack/src/analysis/tests/test_layout.rs b/crates/sqlquack/src/analysis/tests/test_layout.rs new file mode 100644 index 000000000..fefb8fc4a --- /dev/null +++ b/crates/sqlquack/src/analysis/tests/test_layout.rs @@ -0,0 +1,126 @@ +use sqlquack_blocks::layout::main::checked_layout::checked_layout; +use sqlquack_blocks::layout::main::layout_sql::layout_sql; +use sqlquack_blocks::macros::main::all_helper_statements::all_helper_statements; +use sqlquack_format::FormatDialect; + +use crate::analysis::tests::helpers::laid_out_creation_failure; +use crate::analysis::tests::test_types::{ + LayoutCreationTestCase, LayoutFallbackTestCase, LayoutTestCase, +}; + +#[test] +fn given_converted_sql_when_laid_out_then_long_lines_break_and_tokens_keep_their_case() { + let test_cases = [ + LayoutTestCase { + description: "a short query stays on one line", + sql: r#"SELECT CAST(1 AS DECIMAL(1,0)) AS "1""#, + expected_layout: r#"SELECT CAST(1 AS DECIMAL(1, 0)) AS "1""#, + }, + LayoutTestCase { + description: "a wide decimal division breaks its HUGEINT casts and its `//` over indented lines", + sql: r#"SELECT "ORDERS"."ORDER_ID" AS "ORDER_ID", CAST(CAST((sign(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38,2)) * 100000000 AS HUGEINT)) * ((2 * abs(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38,2)) * 100000000 AS HUGEINT)) + 3) // 6)) AS DECIMAL(38,0)) * 0.00000001 AS DECIMAL(18,8)) AS "THIRD" FROM "ORDERS" AS "ORDERS""#, + expected_layout: r#"SELECT + "ORDERS"."ORDER_ID" AS "ORDER_ID", + CAST(CAST(( + sign(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38, 2)) * 100000000 AS HUGEINT)) + * ( + ( + 2 * abs(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38, 2)) * 100000000 AS HUGEINT)) + 3 + ) + // 6 + ) + ) AS DECIMAL(38, 0)) + * 0.00000001 AS DECIMAL(18, 8)) AS "THIRD" +FROM "ORDERS" AS "ORDERS""#, + }, + LayoutTestCase { + description: "a long CASE in a lambda breaks at its branches", + sql: r#"SELECT list_transform([1], lambda x: CASE WHEN abs(x) > 9007199254740992 THEN CAST('a long raise message' AS INTEGER) ELSE x END)[1] AS "E" FROM "T" AS "T""#, + expected_layout: r#"SELECT + list_transform( + [1], + lambda x: CASE + WHEN abs(x) > 9007199254740992 + THEN CAST('a long raise message' AS INTEGER) + ELSE x + END + )[1] AS "E" +FROM "T" AS "T""#, + }, + LayoutTestCase { + description: "a macro keeps CREATE OR REPLACE together, and a lambda body that only starts with a parenthesis stays inline", + sql: r#"CREATE OR REPLACE MACRO sqlquack.quotient_v1(quack_dividend, quack_divisor, quack_power) AS list_transform([quack_dividend % quack_divisor], lambda quack_rest: (quack_dividend // quack_divisor) * quack_power + quack_rest // quack_divisor)[1]"#, + expected_layout: r#"CREATE OR REPLACE MACRO sqlquack.quotient_v1( + quack_dividend, + quack_divisor, + quack_power +) AS list_transform( + [quack_dividend % quack_divisor], + lambda quack_rest: (quack_dividend // quack_divisor) * quack_power + quack_rest // quack_divisor +)[1]"#, + }, + LayoutTestCase { + description: "keywords in strings and quoted names, and doubled spaces in a dollar-quoted string, are kept", + sql: r#"SELECT DISTINCT * EXCEPT ("B"), 'x FROM y, z' AS "WHERE, FROM", $$a FROM b, WHERE c$$ AS "TEXT" FROM "T" AS "T" WHERE ("T"."A" IS DISTINCT FROM "T"."B")"#, + expected_layout: r#"SELECT DISTINCT + * EXCEPT ("B"), + 'x FROM y, z' AS "WHERE, FROM", + $$a FROM b, WHERE c$$ AS "TEXT" +FROM "T" AS "T" +WHERE ("T"."A" IS DISTINCT FROM "T"."B")"#, + }, + ]; + for test_case in test_cases { + let laid_out = checked_layout(test_case.sql, FormatDialect::DuckDB); + assert_eq!( + laid_out.as_deref(), + Ok(test_case.expected_layout), + "{}", + test_case.description + ); + } +} + +#[test] +fn given_sql_the_formatter_refuses_when_laid_out_then_the_single_line_is_kept() { + let test_cases = [LayoutFallbackTestCase { + description: "SQL that does not parse", + sql: r#"SELECT ("T"."A" + FROM "T" AS "T""#, + expected_reason: "the formatter refused it", + }]; + for test_case in test_cases { + let refusal = + checked_layout(test_case.sql, FormatDialect::DuckDB).expect_err(test_case.description); + assert!( + refusal.contains(test_case.expected_reason), + "{}: {refusal}", + test_case.description + ); + assert_eq!( + layout_sql(test_case.sql, FormatDialect::DuckDB), + test_case.sql, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_every_helper_macro_when_laid_out_then_duckdb_creates_it() { + let test_cases = [LayoutCreationTestCase { + description: "every helper laid out over lines, as convert prints it, is created", + expected_failures: vec![], + }]; + for test_case in test_cases { + let connection = duckdb::Connection::open_in_memory().expect("DuckDB opens"); + let failures: Vec = all_helper_statements() + .iter() + .filter_map(|statement| laid_out_creation_failure(&connection, statement).err()) + .collect(); + assert_eq!( + failures, test_case.expected_failures, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack/src/analysis/tests/test_types.rs b/crates/sqlquack/src/analysis/tests/test_types.rs index 6d2bc948e..414bc92fe 100644 --- a/crates/sqlquack/src/analysis/tests/test_types.rs +++ b/crates/sqlquack/src/analysis/tests/test_types.rs @@ -235,3 +235,23 @@ pub(crate) struct EverydayQuery { #[serde(default)] pub(crate) uncertain: bool, } + +/// Single-line converted SQL and its expected layout. +pub(crate) struct LayoutTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_layout: &'static str, +} + +/// SQL the formatter refuses, and the reason its check gives. +pub(crate) struct LayoutFallbackTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_reason: &'static str, +} + +/// Helper macros laid out and created in DuckDB, and the failures expected. +pub(crate) struct LayoutCreationTestCase { + pub(crate) description: &'static str, + pub(crate) expected_failures: Vec, +} diff --git a/crates/sqlquack/src/cli/_helpers/convert_command.rs b/crates/sqlquack/src/cli/_helpers/convert_command.rs index 8ca1fcdf4..345e7d34f 100644 --- a/crates/sqlquack/src/cli/_helpers/convert_command.rs +++ b/crates/sqlquack/src/cli/_helpers/convert_command.rs @@ -16,6 +16,7 @@ use sqlquack_diagnostics::diagnostics::main::warning_at::warning_at; use sqlquack_diagnostics::diagnostics::models::{Diagnostic, Palette}; use sqlquack_diagnostics::provenance::main::merge_provenance::merge_provenance; use sqlquack_diagnostics::refusals::models::Refusal; +use sqlquack_format::FormatDialect; use sqlquack_policy::config::constants::CONFIG_FILE_NAME; use sqlquack_policy::config::main::load_config::load_config; use sqlquack_policy::config::models::UserConfig; @@ -575,7 +576,7 @@ pub(crate) fn chosen_options( ) } -/// The report of a finished analysis of `sql`, its SQL one clause per line unless `compact`. +/// The report of a finished analysis of `sql`, its SQL and helper macros laid out over lines unless `compact`. pub(crate) fn analysis_report( analysis: &Analysis, (sql, compact): (&str, bool), @@ -626,16 +627,22 @@ pub(crate) fn analysis_report( .iter() .map(|done| match compact { true => done.sql.clone(), - false => layout_sql(&done.sql), + false => layout_sql(&done.sql, FormatDialect::DuckDB), }) .collect::>() .join(";\n"), false => String::new(), }; - let macros = with_helpers( + let macros: Vec = with_helpers( override_macros(&provenance.divergences, options), &converted_sql, - ); + ) + .into_iter() + .map(|statement| match compact { + true => statement, + false => layout_sql(&statement, FormatDialect::DuckDB), + }) + .collect(); Ok(ConvertReport { converted, profile, diff --git a/crates/sqlquack/tests/cli/helpers.rs b/crates/sqlquack/tests/cli/helpers.rs index faecca08a..3db46e46a 100644 --- a/crates/sqlquack/tests/cli/helpers.rs +++ b/crates/sqlquack/tests/cli/helpers.rs @@ -819,9 +819,11 @@ pub(crate) fn run_convert(sql: &str, flags: &[&str], mention: &str) -> ConvertOu ConvertOutcome { exit_code: output.status.code(), printed_sql: stdout - .lines() - .find(|line| !line.starts_with("CREATE ")) - .is_some_and(|line| line.starts_with("SELECT") || line.starts_with("WITH")), + .split(";\n") + .find(|statement| !statement.starts_with("CREATE ")) + .is_some_and(|statement| { + statement.starts_with("SELECT") || statement.starts_with("WITH") + }), json_converted: json["converted"].as_bool(), stderr_mentions: String::from_utf8_lossy(&output.stderr).contains(mention), } diff --git a/crates/sqlquack/tests/cli/test_convert.rs b/crates/sqlquack/tests/cli/test_convert.rs index 489f0c368..c2839610a 100644 --- a/crates/sqlquack/tests/cli/test_convert.rs +++ b/crates/sqlquack/tests/cli/test_convert.rs @@ -1655,10 +1655,10 @@ fn given_refused_sql_when_converting_then_text_and_json_carry_the_same_coded_dia fn given_layout_flags_when_converting_then_clauses_break_unless_compact_in_text_and_json() { let test_cases = [ LayoutFlagsTestCase { - description: "each clause and select item on its own line, in text and in --json", + description: "a statement too long for one line breaks at its clauses and items, in text and in --json", flags: &[], - expected_sql: "SELECT\n CAST(1 AS DECIMAL(1,0)) AS \"A\",\n CAST(2 AS DECIMAL(1,0)) AS \"B\",\n \ - 'some longer text here' AS \"C\"\nWHERE (CAST(1 AS DECIMAL(1,0)) = CAST(1 AS DECIMAL(1,0)))", + expected_sql: "SELECT\n CAST(1 AS DECIMAL(1, 0)) AS \"A\",\n CAST(2 AS DECIMAL(1, 0)) AS \"B\",\n \ + 'some longer text here' AS \"C\"\nWHERE (CAST(1 AS DECIMAL(1, 0)) = CAST(1 AS DECIMAL(1, 0)))", }, LayoutFlagsTestCase { description: "--compact prints the statement on one line, in text and in --json", diff --git a/website/public/index.html b/website/public/index.html index 19c13c263..24efa998c 100644 --- a/website/public/index.html +++ b/website/public/index.html @@ -88,10 +88,11 @@

Run warehouse SQL locally.

sqlquack convert revenue.sql SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", - CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY "ORDERS"."CUSTOMER_ID" +GROUP BY + "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted @@ -159,10 +160,16 @@

DuckDB SQL that keeps Snowflake's meaning

sqlquack convert div.sql --no-config SELECT "ORDERS"."ORDER_ID" AS "ORDER_ID", - CAST(CAST((sign(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38,2)) - * 100000000 AS HUGEINT)) * ((2 * abs(…) + 3) // 6)) - AS DECIMAL(38,0)) * CAST('0.00000001' AS DECIMAL(38,8)) - AS DECIMAL(18,8)) AS "THIRD" + CAST(CAST(( + sign(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38, 2)) * 100000000 AS HUGEINT)) + * ( + ( + 2 * abs(CAST(CAST("ORDERS"."TOTAL" AS DECIMAL(38, 2)) * 100000000 AS HUGEINT)) + 3 + ) + // 6 + ) + ) AS DECIMAL(38, 0)) + * 0.00000001 AS DECIMAL(18, 8)) AS "THIRD" FROM "ORDERS" AS "ORDERS" sqlquack convert: done, converted @@ -305,7 +312,7 @@

Start in a minute

error[SQ0203]: QUALIFY needs a window function … echo "SELECT 1 AS total" | sqlquack convert --no-config -SELECT CAST(1 AS DECIMAL(1,0)) AS "TOTAL" +SELECT CAST(1 AS DECIMAL(1, 0)) AS "TOTAL" sqlquack convert: done, converted diff --git a/website/src/content/docs/docs/cli/convert.mdx b/website/src/content/docs/docs/cli/convert.mdx index c0958f9c9..4241e3c72 100644 --- a/website/src/content/docs/docs/cli/convert.mdx +++ b/website/src/content/docs/docs/cli/convert.mdx @@ -29,10 +29,11 @@ the known differences and assumptions it relies on, and refusals go to standard $ sqlquack convert revenue.sql --no-config SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", - CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY "ORDERS"."CUSTOMER_ID" +GROUP BY + "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` @@ -78,7 +79,7 @@ With `--json`, `convert` prints one object: { "converted": true, "profile": "utc-v1", - "sql": "SELECT CAST(1 AS DECIMAL(1,0)) AS \"TOTAL\"", + "sql": "SELECT CAST(1 AS DECIMAL(1, 0)) AS \"TOTAL\"", "refusals": [], "divergences": {}, "assumptions": [], diff --git a/website/src/content/docs/docs/helper-macros.mdx b/website/src/content/docs/docs/helper-macros.mdx index c72559652..f611b1f19 100644 --- a/website/src/content/docs/docs/helper-macros.mdx +++ b/website/src/content/docs/docs/helper-macros.mdx @@ -14,7 +14,10 @@ CREATE SCHEMA IF NOT EXISTS sqlquack; CREATE OR REPLACE MACRO sqlquack.json_escaped_v1(quack_text) AS ...; CREATE OR REPLACE MACRO sqlquack.variant_text_v1(quack_site, quack_value) AS ...; SELECT - sqlquack.variant_text_v1('#0d01742fedea', json_extract("EVENTS"."PAYLOAD", '$."customer_id"')) AS "CUSTOMER_ID" + sqlquack.variant_text_v1( + '#0d01742fedea', + json_extract("EVENTS"."PAYLOAD", '$."customer_id"') + ) AS "CUSTOMER_ID" FROM ... ``` diff --git a/website/src/content/docs/docs/index.mdx b/website/src/content/docs/docs/index.mdx index fae53ec5e..2c1011a58 100644 --- a/website/src/content/docs/docs/index.mdx +++ b/website/src/content/docs/docs/index.mdx @@ -30,10 +30,11 @@ Once `sources/orders.yml` declares `ORDERS`, the same file converts: $ sqlquack convert revenue.sql --no-config SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", - CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY "ORDERS"."CUSTOMER_ID" +GROUP BY + "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/policy.mdx b/website/src/content/docs/docs/policy.mdx index b8dc4b55a..d6bb08aff 100644 --- a/website/src/content/docs/docs/policy.mdx +++ b/website/src/content/docs/docs/policy.mdx @@ -104,7 +104,10 @@ error, and `--verbose` adds a warning at each use: $ sqlquack convert due.sql SELECT "ORDERS"."ORDER_ID" AS "ORDER_ID", - CAST(CAST(hash(CAST("ORDERS"."CUSTOMER_ID" AS HUGEINT)) AS HUGEINT) - 9223372036854775808 AS DECIMAL(19,0)) AS "BUCKET" + CAST( + CAST(hash(CAST("ORDERS"."CUSTOMER_ID" AS HUGEINT)) AS HUGEINT) + - 9223372036854775808 AS DECIMAL(19, 0) + ) AS "BUCKET" FROM "ORDERS" AS "ORDERS" sqlquack convert: done, converted (1 known difference allowed: hash — sqlquack explain hash) ``` diff --git a/website/src/content/docs/docs/project.mdx b/website/src/content/docs/docs/project.mdx index e73c70150..ab99247ff 100644 --- a/website/src/content/docs/docs/project.mdx +++ b/website/src/content/docs/docs/project.mdx @@ -28,9 +28,10 @@ $ sqlquack convert revenue.sql sqlquack convert: converting… SELECT "ORDERS"."STATUS" AS "STATUS", - CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" -GROUP BY "ORDERS"."STATUS" +GROUP BY + "ORDERS"."STATUS" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/python.mdx b/website/src/content/docs/docs/python.mdx index 210ead0d3..2ffcb3c89 100644 --- a/website/src/content/docs/docs/python.mdx +++ b/website/src/content/docs/docs/python.mdx @@ -14,7 +14,7 @@ sqlquack.check(sql="SELECT 1 QUALIFY TRUE").diagnostics[0].code # "SQ0203" result = sqlquack.convert( sql="SELECT 1 AS total", options=sqlquack.ConvertOptions(no_config=True) ) -result.sql # 'SELECT CAST(1 AS DECIMAL(1,0)) AS "TOTAL"' +result.sql # 'SELECT CAST(1 AS DECIMAL(1, 0)) AS "TOTAL"' ``` ## Install diff --git a/website/src/content/docs/docs/quickstart.mdx b/website/src/content/docs/docs/quickstart.mdx index 8c6652efe..fee7bcc9b 100644 --- a/website/src/content/docs/docs/quickstart.mdx +++ b/website/src/content/docs/docs/quickstart.mdx @@ -94,10 +94,11 @@ sources: $ sqlquack convert revenue.sql --no-config SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", - CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY "ORDERS"."CUSTOMER_ID" +GROUP BY + "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/sessions.mdx b/website/src/content/docs/docs/sessions.mdx index 84a188632..a2ecb82d4 100644 --- a/website/src/content/docs/docs/sessions.mdx +++ b/website/src/content/docs/docs/sessions.mdx @@ -39,7 +39,8 @@ A `TIMEZONE` other than UTC refuses them, in every mode, and a note says so: ```console $ sqlquack convert orders.sql --session TIMEZONE=Europe/London -SELECT "ORDERS"."ORDER_ID" AS "ORDER_ID" FROM "ORDERS" AS "ORDERS" +SELECT "ORDERS"."ORDER_ID" AS "ORDER_ID" +FROM "ORDERS" AS "ORDERS" sqlquack convert: note: the session time zone is Europe/London, not UTC, so statements that depend on it are refused sqlquack convert: done, converted ``` diff --git a/website/tests/playground.test.mjs b/website/tests/playground.test.mjs index a100faaf6..25a40ccc7 100644 --- a/website/tests/playground.test.mjs +++ b/website/tests/playground.test.mjs @@ -31,8 +31,10 @@ async function pageDuckDB() { for (const extension of DUCKDB_EXTENSIONS) conn.query(`LOAD ${extension}`); for (const table of JSON.parse(fixture_tables(PAGE_SOURCES)).tables) conn.query(table); conn.query(ROWS); - // Every value as text, as the page reads them. - return (sql) => conn.query(`SELECT COLUMNS(*)::VARCHAR FROM (${sql}) AS result`).toArray().map((row) => Object.values(row.toJSON())); + // Every value as text, as the page reads them; `execute` runs a statement such as a helper macro's as it is. + const run = (sql) => conn.query(`SELECT COLUMNS(*)::VARCHAR FROM (${sql}) AS result`).toArray().map((row) => Object.values(row.toJSON())); + run.execute = (sql) => conn.query(sql); + return run; } const marked = (html) => [...html.matchAll(/]*>(.*?)<\/mark>/g)].map((match) => match[1]); @@ -179,18 +181,31 @@ test('a run error from the in-browser DuckDB is told apart from an error about t assert.ok(!previewLimit('Invalid Input Error: Attempting to fetch from an unsuccessful query result')); }); -test('translated SQL is laid out one clause per line unless the compact setting asks for one line', () => { +test('translated SQL and its helper macros are laid out over lines unless the compact setting asks for one line', async () => { const sql = "SELECT order_id, note, 'a longer literal' AS label FROM orders WHERE order_id > 1"; - const translated = (settings) => JSON.parse(analyze(sql, SOURCES, JSON.stringify(settings))).translate.statements[0].sql; - const laidOut = translated({}); + const translated = (sql, settings) => JSON.parse(analyze(sql, SOURCES, JSON.stringify(settings))).translate.statements[0]; + const laidOut = translated(sql, {}).sql; assert.equal( laidOut, 'SELECT\n "ORDERS"."ORDER_ID" AS "ORDER_ID",\n "ORDERS"."NOTE" AS "NOTE",\n ' + - '\'a longer literal\' AS "LABEL"\nFROM "ORDERS" AS "ORDERS"\nWHERE ("ORDERS"."ORDER_ID" > CAST(1 AS DECIMAL(1,0)))', + '\'a longer literal\' AS "LABEL"\nFROM "ORDERS" AS "ORDERS"\nWHERE ("ORDERS"."ORDER_ID" > CAST(1 AS DECIMAL(1, 0)))', ); - const compact = translated({ compact: true }); - assert.equal(compact, laidOut.replace(/\n */g, ' ')); + // Only whitespace differs from the compact SQL. + const compact = translated(sql, { compact: true }).sql; assert.ok(!compact.includes('\n')); + assert.equal(compact.replace(/\s+/g, ''), laidOut.replace(/\s+/g, '')); + // A wide division calls a helper macro, laid out like the SQL, and the page's DuckDB creates and runs it. + const division = 'SELECT order_id, order_id / 7 AS q FROM orders ORDER BY order_id'; + const [wide, wideCompact] = [translated(division, {}), translated(division, { compact: true })]; + assert.ok(wide.setup.length > 1); + assert.ok(wide.setup.slice(1).every((statement) => statement.includes('\n '))); + assert.ok(wideCompact.setup.every((statement) => !statement.includes('\n'))); + const rows = async (statement) => { + const run = await pageDuckDB(); + for (const helper of statement.setup) run.execute(helper); + return run(statement.sql); + }; + assert.deepEqual(await rows(wide), await rows(wideCompact)); }); test('an undeclared relation is refused by name, and the stub the page adds stays refused until filled in', async () => { diff --git a/website/wasm/Cargo.lock b/website/wasm/Cargo.lock index 286ae0b72..9fa34c46b 100644 --- a/website/wasm/Cargo.lock +++ b/website/wasm/Cargo.lock @@ -224,6 +224,7 @@ version = "0.5.1" dependencies = [ "serde_json", "sqlquack-diagnostics", + "sqlquack-format", "sqlquack-semantics", ] @@ -259,6 +260,15 @@ dependencies = [ "sqlquack-syntax", ] +[[package]] +name = "sqlquack-format" +version = "0.4.1" +dependencies = [ + "serde_json", + "sqlquack-parse", + "sqlquack-syntax", +] + [[package]] name = "sqlquack-parse" version = "0.5.1" @@ -323,6 +333,7 @@ dependencies = [ "sqlquack-blocks", "sqlquack-convert", "sqlquack-diagnostics", + "sqlquack-format", "sqlquack-parse", "sqlquack-policy", "sqlquack-semantics", diff --git a/website/wasm/Cargo.toml b/website/wasm/Cargo.toml index c5dc1f29a..495acd1a0 100644 --- a/website/wasm/Cargo.toml +++ b/website/wasm/Cargo.toml @@ -16,6 +16,7 @@ members = ["."] [dependencies] serde = { version = "1", features = ["derive"] } serde_json = "1" +sqlquack-format = { path = "../../crates/sqlquack-format", features = ["duckdb"] } sqlquack-diagnostics = { path = "../../crates/sqlquack-diagnostics", features = ["snowflake"] } sqlquack-parse = { path = "../../crates/sqlquack-parse" } sqlquack-convert = { path = "../../crates/sqlquack-convert", features = ["snowflake"] } diff --git a/website/wasm/src/lib.rs b/website/wasm/src/lib.rs index 15bb67ae1..d5c346e34 100644 --- a/website/wasm/src/lib.rs +++ b/website/wasm/src/lib.rs @@ -38,6 +38,7 @@ use sqlquack_diagnostics::refusals::main::refuse_unparsed::refuse_unparsed; use sqlquack_diagnostics::refusals::models::{Refusal, RefusalCategory}; use sqlquack_diagnostics::snowflake::classification::main::cast_type_name::cast_type_name; use sqlquack_diagnostics::snowflake::refusals::main::snowflake_rejections::snowflake_rejections; +use sqlquack_format::FormatDialect; use sqlquack_parse::parsing::main::parse_source::parse_source; use sqlquack_policy::known_differences::constants::KNOWN_DIFFERENCES; use sqlquack_semantics::dialects::constants::SNOWFLAKE; @@ -389,7 +390,7 @@ fn stages(sql: &str, catalog: &Catalog, settings: &str) -> Value { Ok(done) => converted.push(json!({ "sql": match settings.compact { true => done.sql.clone(), - false => layout_sql(&done.sql), + false => layout_sql(&done.sql, FormatDialect::DuckDB), }, "columns": done.columns.iter().map(|column| json!({ "name": column.name, @@ -413,7 +414,13 @@ fn stages(sql: &str, catalog: &Catalog, settings: &str) -> Value { // Where each raise ID in the SQL comes from, so a run's error points at the source. "raises": done.raises, // The helper macros the SQL calls, which the page creates before running it. - "setup": helper_statements(&[done.sql.as_str()]), + "setup": helper_statements(&[done.sql.as_str()]) + .into_iter() + .map(|statement| match settings.compact { + true => statement, + false => layout_sql(&statement, FormatDialect::DuckDB), + }) + .collect::>(), })), Err(refusal) => refusals.push(placed(refusal, statement, &lines)), } From 98d44b5075dd4bc96299a7465b50ab1aca618ee1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:15:33 +0000 Subject: [PATCH 4/8] fix(syntax): keep the parentheses of a set operation that starts a longer one ((S1) UNION (S2)) INTERSECT (S3) passed the union to the set-operation parser bare, so Snowflake's INTERSECT precedence moved the INTERSECT inside it and changed the result. The parenthesised union now stays a subquery. --- .../src/parser/_helpers/expressions/paths.rs | 19 ++++++++++++- .../tests/parser/test_fixes.rs | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs index 86d2b25b0..677314319 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/paths.rs @@ -878,7 +878,24 @@ impl Parser { let left = match &result { Expression::Subquery(subq) => Some(subq.this.clone()), Expression::Union(_) | Expression::Intersect(_) | Expression::Except(_) => { - Some(result.clone()) + Some(Expression::Subquery(Box::new(Subquery { + this: result.clone(), + alias: None, + column_aliases: Vec::new(), + alias_explicit_as: false, + alias_keyword: None, + order_by: None, + limit: None, + offset: None, + distribute_by: None, + sort_by: None, + cluster_by: None, + lateral: false, + modifiers_inside: false, + trailing_comments: Vec::new(), + inferred_type: None, + span: SourceSpan::default(), + }))) } _ => None, }; diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs index c2f8315e5..b2559da73 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -1875,6 +1875,33 @@ fn given_a_time_travel_clause_when_parsing_snowflake_then_its_parentheses_close( } } +#[test] +fn given_a_parenthesised_set_operation_before_another_when_parsing_then_it_keeps_its_parentheses() { + let test_cases = [ + ProjectionTestCase { + description: "((S1) UNION (S2)) INTERSECT (S3) intersects the parenthesised union", + sql: "SELECT (((SELECT 2) UNION (SELECT 3)) INTERSECT (SELECT 3))", + pointer: "/paren/this/subquery/this/intersect/left/subquery/this/union/left/subquery/this/select/expressions/0/literal/value", + expected_value: json!("2"), + }, + ProjectionTestCase { + description: "((S1) EXCEPT (S2)) INTERSECT (S3) intersects the parenthesised difference", + sql: "SELECT (((SELECT 2) EXCEPT (SELECT 3)) INTERSECT (SELECT 3))", + pointer: "/paren/this/subquery/this/intersect/left/subquery/this/except/left/subquery/this/select/expressions/0/literal/value", + expected_value: json!("2"), + }, + ]; + + for test_case in test_cases { + assert_eq!( + projection_at(test_case.sql, test_case.pointer), + test_case.expected_value, + "{}", + test_case.description + ); + } +} + #[test] fn given_a_keyword_lambda_parameter_when_parsing_duckdb_then_it_names_the_parameter() { let test_cases = [ From e8570b12149b742c1b76879cfec214ad004d1814 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 10 Nov 2026 16:15:33 +0000 Subject: [PATCH 5/8] fix(format): keep a sign touching a modulo, which DuckDB reads as one operator DuckDB lexes %- and %+ as single operators, so spacing 7%-2 as 7 % -2 would turn an error into a value. The spacing guard now refuses to separate them. --- .../src/duckdb/profile/tests/test_format_sql.rs | 11 ++++++++++- .../sqlquack-format/src/format/_helpers/touching.rs | 8 +++++--- crates/sqlquack-format/src/format/constants.rs | 2 ++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs index 78590a45d..e0bbeff01 100644 --- a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs @@ -1,5 +1,7 @@ use crate::duckdb::profile::tests::test_types; -use crate::format::constants::{DEPTH_FAILURE, JOIN_FAILURE, PARSE_FAILURE, TYPE_DEPTH_FAILURE}; +use crate::format::constants::{ + DEPTH_FAILURE, JOIN_FAILURE, PARSE_FAILURE, SEPARATION_FAILURE, TYPE_DEPTH_FAILURE, +}; use crate::format::models::FormatDialect; use crate::format::tests::helpers::{format_outcome, refusal}; @@ -38,6 +40,13 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re "CREATE macro first_rows(n) AS table SELECT\n *\nFROM t\nLIMIT n", ), }, + test_types::FormatSqlTestCase { + description: "a sign written touching a modulo stays touching, since DuckDB reads `%-` as one operator", + sql: "select 7%-2", + line_width: None, + dialect: FormatDialect::DuckDB, + expected_outcome: Err(SEPARATION_FAILURE), + }, test_types::FormatSqlTestCase { description: "a minus is not joined to a DuckDB `@`, which would read as the operator `-@`", sql: "select - @a", diff --git a/crates/sqlquack-format/src/format/_helpers/touching.rs b/crates/sqlquack-format/src/format/_helpers/touching.rs index 7a1abc30a..337c5315e 100644 --- a/crates/sqlquack-format/src/format/_helpers/touching.rs +++ b/crates/sqlquack-format/src/format/_helpers/touching.rs @@ -8,8 +8,8 @@ use crate::dialects::main::is_keyword::is_keyword; use crate::dialects::models::Lexeme; use crate::format::_helpers::token_layout::AuthoredSql; use crate::format::constants::{ - GUARD_OPERATORS, JOIN_FAILURE, OPERATOR_BYTES, SEPARABLE_KEYWORDS, SEPARABLE_OPERATORS, - SEPARATION_FAILURE, UNSEPARABLE_NEIGHBOURS, + GUARD_OPERATORS, JOIN_FAILURE, MODULO_OPERATOR, OPERATOR_BYTES, SEPARABLE_KEYWORDS, + SEPARABLE_OPERATORS, SEPARATION_FAILURE, UNSEPARABLE_NEIGHBOURS, }; /// Refuse output that separates touching tokens, or joins separated ones, where that could change their reading. @@ -54,7 +54,7 @@ fn separable(previous: &Lexeme, next: &Lexeme) -> bool { || (!numeric && (clause(previous) || clause(next))) } -/// Whether the parse tree's binary `operator` may be spaced from `neighbour`: the guard's own lexing of the source finds exactly it, and neither side is a path or name accessor. +/// Whether the parse tree's binary `operator` may be spaced from `neighbour`: the guard's own lexing finds exactly it, no sign follows a `%` as in DuckDB's `%-`, and neither side is an accessor. fn separable_operator( sql: &str, operator: &Lexeme, @@ -76,8 +76,10 @@ fn separable_operator( && matches!(&bytes[..start], [.., digit, b'e' | b'E'] if digit.is_ascii_digit() || *digit == b'.'); let accessor = UNSEPARABLE_NEIGHBOURS.contains(&neighbour.raw.as_str()) || (neighbour.raw.starts_with(['@', '$']) && !whole_variable(neighbour, rules)); + let signed_modulo = operator.raw == MODULO_OPERATOR && end > operator.end; SEPARABLE_OPERATORS.contains(&operator.raw.as_str()) && !exponent + && !signed_modulo && !accessor && !sql[start..end].contains(':') && operator_pieces(&sql[start..end], start).contains(&(operator.start, operator.end)) diff --git a/crates/sqlquack-format/src/format/constants.rs b/crates/sqlquack-format/src/format/constants.rs index 17c7c09d7..aaacec335 100644 --- a/crates/sqlquack-format/src/format/constants.rs +++ b/crates/sqlquack-format/src/format/constants.rs @@ -203,6 +203,8 @@ pub(crate) const ASSIGNMENT_OPERATOR: &str = "="; pub(crate) const SEPARABLE_OPERATORS: [&str; 15] = [ "=", "<>", "!=", "<", ">", "<=", ">=", "+", "-", "*", "/", "//", "%", "||", "->", ]; +/// The modulo operator, which DuckDB lexes together with a sign written touching it, as `%-`. +pub(crate) const MODULO_OPERATOR: &str = "%"; /// Tokens a binary operator is never spaced from: name and path accessors. pub(crate) const UNSEPARABLE_NEIGHBOURS: [&str; 3] = [".", ":", "::"]; /// Keywords after which a clause word is a table alias instead. From 5a156cb208dcd8a2ebf12be372ddcc3ba71c5c8a Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 04:47:25 +0000 Subject: [PATCH 6/8] feat(format): break wide window specifications and keep one-line clauses on their header A window specification wider than the layout guide now puts PARTITION BY, ORDER BY and its frame each on their own line inside OVER (...), instead of leaving one long line. A narrow window stays inline. A GROUP BY, ORDER BY or other BY clause whose single item fits now stays on the header line (`GROUP BY a`), and a join's ON or USING condition without a top-level AND or OR joins the JOIN line when the result fits. The format corpus is re-recorded: 36 entries change layout, none moves between formatted and refused, and each keeps its earlier layout as previously_formatted. The docs samples are relaid to match. --- .../profile/tests/test_format_sql.rs | 2 +- .../duckdb/profile/tests/test_format_sql.rs | 18 +++ .../src/format/_helpers/guide.rs | 35 +++++- .../src/format/_helpers/printed_layout.rs | 81 +++++++++++++- .../sqlquack-format/src/format/constants.rs | 6 + .../tests/fixtures/FORMAT_CORPUS_NOTICE.md | 2 +- .../format/tests/fixtures/format_corpus.json | 104 ++++++++++++------ .../profile/tests/test_format_sql.rs | 10 +- .../profile/tests/test_format_corpus.rs | 12 +- .../profile/tests/test_format_sql.rs | 10 +- website/src/content/docs/docs/cli/convert.mdx | 3 +- website/src/content/docs/docs/index.mdx | 3 +- website/src/content/docs/docs/project.mdx | 3 +- website/src/content/docs/docs/quickstart.mdx | 3 +- 14 files changed, 221 insertions(+), 71 deletions(-) diff --git a/crates/sqlquack-format/src/databricks/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/databricks/profile/tests/test_format_sql.rs index d2988cd79..e009facc6 100644 --- a/crates/sqlquack-format/src/databricks/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/databricks/profile/tests/test_format_sql.rs @@ -25,7 +25,7 @@ fn given_databricks_sql_when_formatting_then_it_is_laid_out_or_refused() { sql: "select a from t distribute by a sort by a", line_width: None, dialect: FormatDialect::Databricks, - expected_outcome: Ok("SELECT a\nFROM t\nDISTRIBUTE BY\n a\nSORT BY\n a"), + expected_outcome: Ok("SELECT a\nFROM t\nDISTRIBUTE BY a\nSORT BY a"), }, test_types::FormatSqlTestCase { description: "a Databricks path, bracket path and try-cast stay touching", diff --git a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs index e0bbeff01..b4fa6b7d7 100644 --- a/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/duckdb/profile/tests/test_format_sql.rs @@ -122,6 +122,24 @@ fn given_constructs_sqlquack_converts_to_when_formatting_then_they_are_laid_out( "SELECT\n x IN ((\n (\n SELECT 1\n ) UNION ALL (\n SELECT 2\n )\n ) UNION ALL (\n SELECT 3\n ))\nFROM t", ), }, + test_types::FormatSqlTestCase { + description: "a wide window puts each of its clauses on its own line, a narrow one stays inline", + sql: "select sum(amount) over (partition by customer_id, region order by ordered_at rows between unbounded preceding and current row) as running, sum(x) over (order by y) as s from orders", + line_width: Some(100), + dialect: FormatDialect::DuckDB, + expected_outcome: Ok( + "SELECT\n SUM(amount) OVER (\n PARTITION BY customer_id, region\n ORDER BY ordered_at\n ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW\n ) AS running,\n SUM(x) OVER (ORDER BY y) AS s\nFROM orders", + ), + }, + test_types::FormatSqlTestCase { + description: "a single-item GROUP BY or ORDER BY and a join's condition without AND or OR stay on their header line", + sql: "select a from t left join u on (t.id = u.id) join v on v.id = t.id and v.k = 1 group by a order by a desc", + line_width: Some(100), + dialect: FormatDialect::DuckDB, + expected_outcome: Ok( + "SELECT a\nFROM t\nLEFT JOIN u ON (t.id = u.id)\nJOIN v\n ON v.id = t.id AND v.k = 1\nGROUP BY a\nORDER BY a DESC", + ), + }, test_types::FormatSqlTestCase { description: "a wrapped macro keeps CREATE OR REPLACE together and a lambda body that starts with a parenthesis inline", sql: "create or replace macro m(a, b) as list_transform([a], lambda r: (a // b) * r + a // b)[1]", diff --git a/crates/sqlquack-format/src/format/_helpers/guide.rs b/crates/sqlquack-format/src/format/_helpers/guide.rs index e77e6a810..8e750eb0c 100644 --- a/crates/sqlquack-format/src/format/_helpers/guide.rs +++ b/crates/sqlquack-format/src/format/_helpers/guide.rs @@ -20,7 +20,7 @@ use crate::format::constants::{ PLACEHOLDER_WIDTH, POSTFIX_CAST_WIDTH, REFERENCES_KEYWORD, SAMPLE_KEYWORDS, SAMPLE_ROW_KEYWORD, SEMANTIC_VIEW_CALL, SEMANTIC_VIEW_CLAUSES, SET_OPERATOR_MODIFIERS, SET_OPERATORS, SPACED_BEFORE_PARENTHESIS, STRUCT_TYPE_WORDS, TYPE_CONTINUATIONS, TYPED_LITERALS, - VALUE_TABLE_KINDS, VALUES_KEYWORD, + VALUE_TABLE_KINDS, VALUES_KEYWORD, WINDOW_CLAUSE_STARTS, WINDOW_KEYWORD, }; /// The guide's text and its tokens, with byte ranges in that text. @@ -1302,6 +1302,8 @@ impl<'a> Guide<'a> { self.expr(opener + 1, closer - 1, indent); } else if self.quantified_pattern(opener) { self.list(closer, &[(opener + 1, closer - 1)], true, indent); + } else if self.is(previous, WINDOW_KEYWORD) { + self.window(opener, closer, indent); } else if self.is(previous, "PIVOT") || self.is(previous, "UNPIVOT") || self.enclosing[opener] @@ -1330,6 +1332,37 @@ impl<'a> Guide<'a> { } } + /// Lay out a window specification: past the guide width, each of its clauses on its own line. + fn window(&mut self, opener: usize, closer: usize, indent: usize) { + if opener + 1 >= closer || self.compact(opener + 1, closer - 1) <= GUIDE_WIDTH { + self.expr(opener + 1, closer - 1, indent); + return; + } + let mut starts: Vec = vec![opener + 1]; + starts.extend( + self.tops(opener + 2, closer - 1) + .into_iter() + .filter(|&index| { + self.is_any(index, &WINDOW_CLAUSE_STARTS) + && (self.is(index + 1, "BY") + || !(self.is(index, "PARTITION") || self.is(index, "ORDER"))) + }), + ); + let items: Vec<(usize, usize)> = starts + .iter() + .enumerate() + .map(|(position, &start)| { + ( + start, + starts + .get(position + 1) + .map_or(closer - 1, |&next| next - 1), + ) + }) + .collect(); + self.list(closer, &items, true, indent); + } + /// Lay out a tuple, split past the guide width, or a parenthesised expression, always split. fn tuple(&mut self, opener: usize, closer: usize, indent: usize) { let items = self.split_commas(opener + 1, closer - 1); diff --git a/crates/sqlquack-format/src/format/_helpers/printed_layout.rs b/crates/sqlquack-format/src/format/_helpers/printed_layout.rs index 5f7fea3a1..50eda7e3f 100644 --- a/crates/sqlquack-format/src/format/_helpers/printed_layout.rs +++ b/crates/sqlquack-format/src/format/_helpers/printed_layout.rs @@ -16,7 +16,23 @@ const BETWEEN_SEPARATOR: &str = "AND"; const VALUES_KEYWORD: &str = "VALUES"; const DECODE_FUNCTION: &str = "DECODE"; const CASE_KEYWORD: &str = "CASE"; +const JOIN_KEYWORD: &str = "JOIN"; const SINGLE_ITEM_CLAUSES: [&str; 4] = ["SELECT", "WHERE", "QUALIFY", "HAVING"]; +/// Words whose clause header is ` BY`, such as GROUP BY. +const BY_CLAUSE_WORDS: [&str; 6] = [ + "GROUP", + "ORDER", + "PARTITION", + "CLUSTER", + "DISTRIBUTE", + "SORT", +]; +/// Keywords that start a join's condition, which joins its line when both fit. +const JOIN_CONDITIONS: [&str; 2] = ["ON", "USING"]; +/// Tokens at a condition's depth that show, before any JOIN, that it is not a join's condition. +const NOT_JOIN_CONDITION: [&str; 9] = [ + "FROM", "WHERE", "SELECT", "ON", "USING", "INTO", "INDEX", "TABLE", ",", +]; const SELECT_MODIFIERS: [&str; 10] = [ "DISTINCT", "ALL", "TOP", "PERCENT", "WITH", "TIES", "ON", "AS", "STRUCT", "VALUE", ]; @@ -422,10 +438,18 @@ impl PrintedSql<'_> { } } for index in 0..self.len() { - if SINGLE_ITEM_CLAUSES.contains(&self.upper(index).as_str()) { + let word = self.upper(index); + if SINGLE_ITEM_CLAUSES.contains(&word.as_str()) + || (BY_CLAUSE_WORDS.contains(&word.as_str()) && self.is(index + 1, "BY")) + { self.join_single_line_body(index, line_width); } } + for index in 1..self.len() { + if JOIN_CONDITIONS.contains(&self.upper(index).as_str()) { + self.join_condition_line(index, line_width); + } + } } /// Start the next line with any binary operator whose operand spans several lines. @@ -595,6 +619,11 @@ impl PrintedSql<'_> { let (first, last) = self.line_of(clause); let body = last + 1; let is_select = self.is(clause, "SELECT"); + let header_last = if self.is(clause + 1, "BY") { + clause + 1 + } else { + clause + }; let modifiers_only = (clause + 1..=last).all(|index| { SELECT_MODIFIERS.contains(&self.upper(index).as_str()) || self.kinds[index] == TokenType::Number @@ -603,7 +632,7 @@ impl PrintedSql<'_> { }); if body >= self.len() || clause != first - || (!is_select && clause != last) + || (!is_select && header_last != last) || (is_select && !modifiers_only) || !self.gaps[body].contains('\n') || !self.gaps[body].trim().is_empty() @@ -645,6 +674,54 @@ impl PrintedSql<'_> { } } + /// Move a join's one-line ON or USING condition onto the line before it when both fit. + fn join_condition_line(&mut self, condition: usize, line_width: usize) { + let (first, last) = self.line_of(condition); + if condition != first + || !self.gaps[condition].contains('\n') + || !self.gaps[condition].trim().is_empty() + || !self.conditions_join(condition) + { + return; + } + let indentation = self.indentation(condition); + if last + 1 < self.len() && self.indentation(last + 1) >= indentation { + return; + } + let (previous, _) = self.line_of(condition - 1); + let previous_indentation = self.indentation(previous); + if previous_indentation >= indentation || self.has_top_level_chain(condition + 1, last) { + return; + } + if self + .joined_width(previous, last, previous_indentation) + .is_some_and(|width| width <= line_width) + { + self.gaps[condition] = SINGLE_SPACE.to_string(); + } + } + + /// Whether the ON or USING at `condition` belongs to a JOIN rather than another statement. + fn conditions_join(&self, condition: usize) -> bool { + let depth = self.depths[condition]; + for index in (0..condition).rev() { + if self.depths[index] < depth { + return false; + } + if self.depths[index] > depth { + continue; + } + let word = self.upper(index); + if word == JOIN_KEYWORD { + return true; + } + if NOT_JOIN_CONDITION.contains(&word.as_str()) { + return false; + } + } + false + } + fn has_top_level_chain(&self, first: usize, last: usize) -> bool { let depth = (first..=last).map(|index| self.depths[index]).min(); (first..=last).any(|index| { diff --git a/crates/sqlquack-format/src/format/constants.rs b/crates/sqlquack-format/src/format/constants.rs index aaacec335..3ca4820c3 100644 --- a/crates/sqlquack-format/src/format/constants.rs +++ b/crates/sqlquack-format/src/format/constants.rs @@ -52,6 +52,12 @@ pub(crate) const INDEX_KEYWORD: &str = "INDEX"; /// The SQL Server keyword that, after CROSS or OUTER, applies a table-valued expression like a join. pub(crate) const APPLY_KEYWORD: &str = "APPLY"; +/// The keyword before the parentheses of a window specification, as in `SUM(x) OVER (...)`. +pub(crate) const WINDOW_KEYWORD: &str = "OVER"; +/// Words that start a clause of a window specification; PARTITION and ORDER only before BY. +pub(crate) const WINDOW_CLAUSE_STARTS: [&str; 5] = + ["PARTITION", "ORDER", "ROWS", "RANGE", "GROUPS"]; + /// The line width past which the layout guide splits a call's arguments or a CASE. pub(crate) const GUIDE_WIDTH: usize = 80; /// Spaces per indent step of the layout guide; printing rescales it to the requested indent. diff --git a/crates/sqlquack-format/src/format/tests/fixtures/FORMAT_CORPUS_NOTICE.md b/crates/sqlquack-format/src/format/tests/fixtures/FORMAT_CORPUS_NOTICE.md index 39e0868f7..e9d6d8fa5 100644 --- a/crates/sqlquack-format/src/format/tests/fixtures/FORMAT_CORPUS_NOTICE.md +++ b/crates/sqlquack-format/src/format/tests/fixtures/FORMAT_CORPUS_NOTICE.md @@ -3,7 +3,7 @@ `format_corpus.json` contains SELECT and WITH queries taken from the SQLGlot dialect identity test fixtures for BigQuery, Databricks, DuckDB, PostgreSQL, Snowflake and T-SQL, as extracted by the Polyglot project. One literal was reworded. The expected `formatted` and `previously_formatted` -values are SQLBuild formatter output. +values are output of this formatter and of the SQLBuild formatter it was ported from. The queries are used under the following license: diff --git a/crates/sqlquack-format/src/format/tests/fixtures/format_corpus.json b/crates/sqlquack-format/src/format/tests/fixtures/format_corpus.json index b45dd71a3..e1d48c234 100644 --- a/crates/sqlquack-format/src/format/tests/fixtures/format_corpus.json +++ b/crates/sqlquack-format/src/format/tests/fixtures/format_corpus.json @@ -77,7 +77,8 @@ { "dialect": "bigquery", "sql": "SELECT x, 1 AS y GROUP BY 1 ORDER BY 1", - "formatted": "SELECT\n x,\n 1 AS y\nGROUP BY\n 1\nORDER BY\n 1" + "formatted": "SELECT\n x,\n 1 AS y\nGROUP BY 1\nORDER BY 1", + "previously_formatted": "SELECT\n x,\n 1 AS y\nGROUP BY\n 1\nORDER BY\n 1" }, { "dialect": "bigquery", @@ -262,7 +263,8 @@ { "dialect": "bigquery", "sql": "SELECT y + 1 FROM x GROUP BY y + 1 ORDER BY 1", - "formatted": "SELECT y + 1\nFROM x\nGROUP BY\n y + 1\nORDER BY\n 1" + "formatted": "SELECT y + 1\nFROM x\nGROUP BY y + 1\nORDER BY 1", + "previously_formatted": "SELECT y + 1\nFROM x\nGROUP BY\n y + 1\nORDER BY\n 1" }, { "dialect": "bigquery", @@ -434,13 +436,13 @@ { "dialect": "bigquery", "sql": "SELECT y + 1 z FROM x GROUP BY y + 1 ORDER BY z", - "formatted": "SELECT y + 1 z\nFROM x\nGROUP BY\n y + 1\nORDER BY\n z", + "formatted": "SELECT y + 1 z\nFROM x\nGROUP BY y + 1\nORDER BY z", "previously_formatted": "SELECT\n y + 1 AS z\nFROM x\nGROUP BY\n y + 1\nORDER BY\n z" }, { "dialect": "bigquery", "sql": "SELECT y + 1 z FROM x GROUP BY y + 1", - "formatted": "SELECT y + 1 z\nFROM x\nGROUP BY\n y + 1", + "formatted": "SELECT y + 1 z\nFROM x\nGROUP BY y + 1", "previously_formatted": "SELECT\n y + 1 AS z\nFROM x\nGROUP BY\n y + 1" }, { @@ -498,7 +500,8 @@ { "dialect": "bigquery", "sql": "SELECT * FROM GAP_FILL(TABLE device_data, ts_column => 'time', bucket_width => INTERVAL '1' MINUTE, value_columns => [('signal', 'locf')]) ORDER BY time", - "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY\n time" + "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY time", + "previously_formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY\n time" }, { "dialect": "bigquery", @@ -508,22 +511,26 @@ { "dialect": "bigquery", "sql": "SELECT * FROM GAP_FILL(TABLE device_data, ts_column => 'time', bucket_width => INTERVAL '1' MINUTE, value_columns => [('signal', 'linear')], ignore_null_values => FALSE) ORDER BY time", - "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'linear')],\n ignore_null_values => FALSE\n)\nORDER BY\n time" + "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'linear')],\n ignore_null_values => FALSE\n)\nORDER BY time", + "previously_formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'linear')],\n ignore_null_values => FALSE\n)\nORDER BY\n time" }, { "dialect": "bigquery", "sql": "SELECT * FROM GAP_FILL(TABLE device_data, ts_column => 'time', bucket_width => INTERVAL '1' MINUTE) ORDER BY time", - "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE\n)\nORDER BY\n time" + "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE\n)\nORDER BY time", + "previously_formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE\n)\nORDER BY\n time" }, { "dialect": "bigquery", "sql": "SELECT * FROM GAP_FILL(TABLE device_data, ts_column => 'time', bucket_width => INTERVAL '1' MINUTE, value_columns => [('signal', 'null')], origin => CAST('2023-11-01 09:30:01' AS DATETIME)) ORDER BY time", - "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'null')],\n origin => CAST('2023-11-01 09:30:01' AS DATETIME)\n)\nORDER BY\n time" + "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'null')],\n origin => CAST('2023-11-01 09:30:01' AS DATETIME)\n)\nORDER BY time", + "previously_formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'null')],\n origin => CAST('2023-11-01 09:30:01' AS DATETIME)\n)\nORDER BY\n time" }, { "dialect": "bigquery", "sql": "SELECT * FROM GAP_FILL(TABLE device_data, ts_column => 'time', bucket_width => INTERVAL '1' MINUTE, value_columns => [('signal', 'locf')]) ORDER BY time", - "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY\n time" + "formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY time", + "previously_formatted": "SELECT *\nFROM GAP_FILL(\n TABLE device_data,\n ts_column => 'time',\n bucket_width => INTERVAL '1' MINUTE,\n value_columns => [('signal', 'locf')]\n)\nORDER BY\n time" }, { "dialect": "bigquery", @@ -758,7 +765,8 @@ { "dialect": "bigquery", "sql": "SELECT a, GROUP_CONCAT(b) FROM table GROUP BY a", - "formatted": "SELECT\n a,\n GROUP_CONCAT(b)\nFROM table\nGROUP BY\n a" + "formatted": "SELECT\n a,\n GROUP_CONCAT(b)\nFROM table\nGROUP BY a", + "previously_formatted": "SELECT\n a,\n GROUP_CONCAT(b)\nFROM table\nGROUP BY\n a" }, { "dialect": "bigquery", @@ -913,7 +921,8 @@ { "dialect": "databricks", "sql": "SELECT col1 FROM t CLUSTER BY col1", - "formatted": "SELECT col1\nFROM t\nCLUSTER BY\n col1" + "formatted": "SELECT col1\nFROM t\nCLUSTER BY col1", + "previously_formatted": "SELECT col1\nFROM t\nCLUSTER BY\n col1" }, { "dialect": "databricks", @@ -1023,7 +1032,8 @@ { "dialect": "databricks", "sql": "SELECT test, LISTAGG(email, '') AS Email FROM organizations GROUP BY test", - "formatted": "SELECT\n test,\n LISTAGG(email, '') AS Email\nFROM organizations\nGROUP BY\n test" + "formatted": "SELECT\n test,\n LISTAGG(email, '') AS Email\nFROM organizations\nGROUP BY test", + "previously_formatted": "SELECT\n test,\n LISTAGG(email, '') AS Email\nFROM organizations\nGROUP BY\n test" }, { "dialect": "databricks", @@ -1268,7 +1278,8 @@ { "dialect": "duckdb", "sql": "SELECT * FROM t1 AS a AT (VERSION => 3) JOIN t2 AS b AT (TIMESTAMP => NOW() - INTERVAL '1' WEEK) ON a.id = b.id", - "formatted": "SELECT *\nFROM t1 AS a AT (VERSION => 3)\nJOIN t2 AS b AT (TIMESTAMP => NOW() - INTERVAL '1' WEEK)\n ON a.id = b.id" + "formatted": "SELECT *\nFROM t1 AS a AT (VERSION => 3)\nJOIN t2 AS b AT (TIMESTAMP => NOW() - INTERVAL '1' WEEK) ON a.id = b.id", + "previously_formatted": "SELECT *\nFROM t1 AS a AT (VERSION => 3)\nJOIN t2 AS b AT (TIMESTAMP => NOW() - INTERVAL '1' WEEK)\n ON a.id = b.id" }, { "dialect": "duckdb", @@ -1343,7 +1354,8 @@ { "dialect": "duckdb", "sql": "SELECT * FROM foo ASOF LEFT JOIN bar ON a = b", - "formatted": "SELECT *\nFROM foo\nASOF LEFT JOIN bar\n ON a = b" + "formatted": "SELECT *\nFROM foo\nASOF LEFT JOIN bar ON a = b", + "previously_formatted": "SELECT *\nFROM foo\nASOF LEFT JOIN bar\n ON a = b" }, { "dialect": "duckdb", @@ -1438,7 +1450,8 @@ { "dialect": "duckdb", "sql": "SELECT a, LOGICAL_OR(b) FROM foo GROUP BY a", - "formatted": "SELECT\n a,\n LOGICAL_OR(b)\nFROM foo\nGROUP BY\n a" + "formatted": "SELECT\n a,\n LOGICAL_OR(b)\nFROM foo\nGROUP BY a", + "previously_formatted": "SELECT\n a,\n LOGICAL_OR(b)\nFROM foo\nGROUP BY\n a" }, { "dialect": "duckdb", @@ -1653,12 +1666,14 @@ { "dialect": "duckdb", "sql": "SELECT i FROM RANGE(5) AS _(i) ORDER BY i ASC", - "formatted": "SELECT i\nFROM RANGE(5) AS _(i)\nORDER BY\n i ASC" + "formatted": "SELECT i\nFROM RANGE(5) AS _(i)\nORDER BY i ASC", + "previously_formatted": "SELECT i\nFROM RANGE(5) AS _(i)\nORDER BY\n i ASC" }, { "dialect": "duckdb", "sql": "SELECT i FROM GENERATE_SERIES(12) AS _(i) ORDER BY i ASC", - "formatted": "SELECT i\nFROM GENERATE_SERIES(12) AS _(i)\nORDER BY\n i ASC" + "formatted": "SELECT i\nFROM GENERATE_SERIES(12) AS _(i)\nORDER BY i ASC", + "previously_formatted": "SELECT i\nFROM GENERATE_SERIES(12) AS _(i)\nORDER BY\n i ASC" }, { "dialect": "duckdb", @@ -2307,7 +2322,8 @@ { "dialect": "postgres", "sql": "SELECT SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) - SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS total", - "formatted": "SELECT SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) - SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS total" + "formatted": "SELECT\n SUM(x) OVER (\n PARTITION BY y\n ORDER BY interval\n ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW\n )\n - SUM(x) OVER (\n PARTITION BY y\n ORDER BY interval\n ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW\n ) AS total", + "previously_formatted": "SELECT SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) - SUM(x) OVER (PARTITION BY y ORDER BY interval ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS total" }, { "dialect": "postgres", @@ -2402,7 +2418,8 @@ { "dialect": "postgres", "sql": "SELECT c.oid, n.nspname, c.relname FROM pg_catalog.pg_class AS c LEFT JOIN pg_catalog.pg_namespace AS n ON n.oid = c.relnamespace WHERE c.relname OPERATOR(pg_catalog.~) '^(courses)$' COLLATE pg_catalog.default AND pg_catalog.PG_TABLE_IS_VISIBLE(c.oid) ORDER BY 2, 3", - "formatted": "SELECT\n c.oid,\n n.nspname,\n c.relname\nFROM pg_catalog.pg_class AS c\nLEFT JOIN pg_catalog.pg_namespace AS n\n ON n.oid = c.relnamespace\nWHERE\n c.relname OPERATOR(pg_catalog.~) '^(courses)$' COLLATE pg_catalog.default AND pg_catalog.PG_TABLE_IS_VISIBLE(c.oid)\nORDER BY\n 2,\n 3" + "formatted": "SELECT\n c.oid,\n n.nspname,\n c.relname\nFROM pg_catalog.pg_class AS c\nLEFT JOIN pg_catalog.pg_namespace AS n ON n.oid = c.relnamespace\nWHERE\n c.relname OPERATOR(pg_catalog.~) '^(courses)$' COLLATE pg_catalog.default AND pg_catalog.PG_TABLE_IS_VISIBLE(c.oid)\nORDER BY\n 2,\n 3", + "previously_formatted": "SELECT\n c.oid,\n n.nspname,\n c.relname\nFROM pg_catalog.pg_class AS c\nLEFT JOIN pg_catalog.pg_namespace AS n\n ON n.oid = c.relnamespace\nWHERE\n c.relname OPERATOR(pg_catalog.~) '^(courses)$' COLLATE pg_catalog.default AND pg_catalog.PG_TABLE_IS_VISIBLE(c.oid)\nORDER BY\n 2,\n 3" }, { "dialect": "postgres", @@ -2507,7 +2524,7 @@ { "dialect": "postgres", "sql": "SELECT m.name FROM manufacturers AS m LEFT JOIN LATERAL GET_PRODUCT_NAMES(m.id) pname ON TRUE WHERE pname IS NULL", - "formatted": "SELECT m.name\nFROM manufacturers AS m\nLEFT JOIN LATERAL GET_PRODUCT_NAMES(m.id) pname\n ON TRUE\nWHERE pname IS NULL", + "formatted": "SELECT m.name\nFROM manufacturers AS m\nLEFT JOIN LATERAL GET_PRODUCT_NAMES(m.id) pname ON TRUE\nWHERE pname IS NULL", "previously_formatted": "SELECT\n m.name\nFROM manufacturers AS m\nLEFT JOIN LATERAL GET_PRODUCT_NAMES(m.id) AS pname\n ON TRUE\nWHERE\n pname IS NULL" }, { @@ -2569,7 +2586,8 @@ { "dialect": "postgres", "sql": "SELECT 1 FROM ((VALUES (1)) AS vals(id) LEFT OUTER JOIN tbl ON vals.id = tbl.id)", - "formatted": "SELECT 1\nFROM ((VALUES (1)) AS vals(id)\nLEFT OUTER JOIN tbl\n ON vals.id = tbl.id)" + "formatted": "SELECT 1\nFROM ((VALUES (1)) AS vals(id)\nLEFT OUTER JOIN tbl ON vals.id = tbl.id)", + "previously_formatted": "SELECT 1\nFROM ((VALUES (1)) AS vals(id)\nLEFT OUTER JOIN tbl\n ON vals.id = tbl.id)" }, { "dialect": "postgres", @@ -3962,27 +3980,32 @@ { "dialect": "snowflake", "sql": "SELECT * FROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN ('2023_Q1', '2023_Q2', '2023_Q3', '2023_Q4', '2024_Q1') DEFAULT ON NULL (0)) ORDER BY empid", - "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN ('2023_Q1', '2023_Q2', '2023_Q3', '2023_Q4', '2024_Q1') DEFAULT ON NULL (0))\nORDER BY\n empid" + "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN ('2023_Q1', '2023_Q2', '2023_Q3', '2023_Q4', '2024_Q1') DEFAULT ON NULL (0))\nORDER BY empid", + "previously_formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN ('2023_Q1', '2023_Q2', '2023_Q3', '2023_Q4', '2024_Q1') DEFAULT ON NULL (0))\nORDER BY\n empid" }, { "dialect": "snowflake", "sql": "SELECT * FROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (SELECT DISTINCT quarter FROM ad_campaign_types_by_quarter WHERE television = TRUE ORDER BY quarter)) ORDER BY empid", - "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (SELECT DISTINCT\n quarter\nFROM ad_campaign_types_by_quarter\nWHERE television = TRUE\nORDER BY\n quarter))\nORDER BY\n empid" + "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (SELECT DISTINCT\n quarter\nFROM ad_campaign_types_by_quarter\nWHERE television = TRUE\nORDER BY quarter))\nORDER BY empid", + "previously_formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (SELECT DISTINCT\n quarter\nFROM ad_campaign_types_by_quarter\nWHERE television = TRUE\nORDER BY\n quarter))\nORDER BY\n empid" }, { "dialect": "snowflake", "sql": "SELECT * FROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY ORDER BY quarter)) ORDER BY empid", - "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY ORDER BY quarter))\nORDER BY\n empid" + "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY ORDER BY quarter))\nORDER BY empid", + "previously_formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY ORDER BY quarter))\nORDER BY\n empid" }, { "dialect": "snowflake", "sql": "SELECT * FROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY)) ORDER BY empid", - "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY))\nORDER BY\n empid" + "formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY))\nORDER BY empid", + "previously_formatted": "SELECT *\nFROM quarterly_sales PIVOT(SUM(amount) FOR quarter IN (ANY))\nORDER BY\n empid" }, { "dialect": "snowflake", "sql": "SELECT * FROM DATA AS DATA_L ASOF JOIN DATA AS DATA_R MATCH_CONDITION (DATA_L.VAL > DATA_R.VAL) ON DATA_L.ID = DATA_R.ID", - "formatted": "SELECT *\nFROM DATA AS DATA_L\nASOF JOIN DATA AS DATA_R MATCH_CONDITION (DATA_L.VAL > DATA_R.VAL)\n ON DATA_L.ID = DATA_R.ID" + "formatted": "SELECT *\nFROM DATA AS DATA_L\nASOF JOIN DATA AS DATA_R MATCH_CONDITION (DATA_L.VAL > DATA_R.VAL) ON DATA_L.ID = DATA_R.ID", + "previously_formatted": "SELECT *\nFROM DATA AS DATA_L\nASOF JOIN DATA AS DATA_R MATCH_CONDITION (DATA_L.VAL > DATA_R.VAL)\n ON DATA_L.ID = DATA_R.ID" }, { "dialect": "snowflake", @@ -4025,7 +4048,8 @@ { "dialect": "snowflake", "sql": "SELECT * FROM t1 INNER JOIN t2 USING (t1.col)", - "formatted": "SELECT *\nFROM t1\nINNER JOIN t2\n USING (t1.col)" + "formatted": "SELECT *\nFROM t1\nINNER JOIN t2 USING (t1.col)", + "previously_formatted": "SELECT *\nFROM t1\nINNER JOIN t2\n USING (t1.col)" }, { "dialect": "snowflake", @@ -4537,7 +4561,8 @@ { "dialect": "snowflake", "sql": "SELECT * FROM (SELECT * FROM t1 JOIN t2 ON t1.a = t2.c) TABLESAMPLE BERNOULLI (1)", - "formatted": "SELECT *\nFROM (\n SELECT *\n FROM t1\n JOIN t2\n ON t1.a = t2.c\n) TABLESAMPLE BERNOULLI (1)" + "formatted": "SELECT *\nFROM (\n SELECT *\n FROM t1\n JOIN t2 ON t1.a = t2.c\n) TABLESAMPLE BERNOULLI (1)", + "previously_formatted": "SELECT *\nFROM (\n SELECT *\n FROM t1\n JOIN t2\n ON t1.a = t2.c\n) TABLESAMPLE BERNOULLI (1)" }, { "dialect": "snowflake", @@ -4658,7 +4683,7 @@ { "dialect": "snowflake", "sql": "\n SELECT oldt.* , newt.*\n FROM my_table BEFORE(STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS oldt\n FULL OUTER JOIN my_table AT(STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS newt\n ON oldt.id = newt.id\n WHERE oldt.id IS NULL OR newt.id IS NULL;\n ", - "formatted": "SELECT\n oldt.*,\n newt.*\nFROM my_table BEFORE (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS oldt\nFULL OUTER JOIN my_table AT (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS newt\n ON oldt.id = newt.id\nWHERE\n oldt.id IS NULL\n OR newt.id IS NULL;", + "formatted": "SELECT\n oldt.*,\n newt.*\nFROM my_table BEFORE (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS oldt\nFULL OUTER JOIN my_table AT (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS newt ON oldt.id = newt.id\nWHERE\n oldt.id IS NULL\n OR newt.id IS NULL;", "previously_formatted": "SELECT\n oldt.*,\n newt.*\nFROM my_table BEFORE (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS oldt\nFULL OUTER JOIN my_table AT (STATEMENT => '8e5d0ca9-005e-44e6-b858-a8f5b37c5726') AS newt\n ON oldt.id = newt.id\nWHERE\n oldt.id IS NULL\n OR newt.id IS NULL" }, { @@ -4919,17 +4944,20 @@ { "dialect": "snowflake", "sql": "SELECT * FROM a CROSS DIRECTED JOIN b USING (id)", - "formatted": "SELECT *\nFROM a\nCROSS DIRECTED JOIN b\n USING (id)" + "formatted": "SELECT *\nFROM a\nCROSS DIRECTED JOIN b USING (id)", + "previously_formatted": "SELECT *\nFROM a\nCROSS DIRECTED JOIN b\n USING (id)" }, { "dialect": "snowflake", "sql": "SELECT * FROM a INNER DIRECTED JOIN b USING (id)", - "formatted": "SELECT *\nFROM a\nINNER DIRECTED JOIN b\n USING (id)" + "formatted": "SELECT *\nFROM a\nINNER DIRECTED JOIN b USING (id)", + "previously_formatted": "SELECT *\nFROM a\nINNER DIRECTED JOIN b\n USING (id)" }, { "dialect": "snowflake", "sql": "SELECT * FROM a NATURAL INNER DIRECTED JOIN b USING (id)", - "formatted": "SELECT *\nFROM a\nNATURAL INNER DIRECTED JOIN b\n USING (id)" + "formatted": "SELECT *\nFROM a\nNATURAL INNER DIRECTED JOIN b USING (id)", + "previously_formatted": "SELECT *\nFROM a\nNATURAL INNER DIRECTED JOIN b\n USING (id)" }, { "dialect": "snowflake", @@ -5109,7 +5137,8 @@ { "dialect": "tsql", "sql": "SELECT * FROM taxi ORDER BY 1 OFFSET 0 ROWS FETCH NEXT 3 ROWS ONLY", - "formatted": "SELECT *\nFROM taxi\nORDER BY\n 1\nOFFSET 0 ROWS\nFETCH NEXT 3 ROWS ONLY" + "formatted": "SELECT *\nFROM taxi\nORDER BY 1\nOFFSET 0 ROWS\nFETCH NEXT 3 ROWS ONLY", + "previously_formatted": "SELECT *\nFROM taxi\nORDER BY\n 1\nOFFSET 0 ROWS\nFETCH NEXT 3 ROWS ONLY" }, { "dialect": "tsql", @@ -5139,7 +5168,8 @@ { "dialect": "tsql", "sql": "SELECT x FROM @MyTableVar AS m JOIN Employee ON m.EmployeeID = Employee.EmployeeID", - "formatted": "SELECT x\nFROM @MyTableVar AS m\nJOIN Employee\n ON m.EmployeeID = Employee.EmployeeID" + "formatted": "SELECT x\nFROM @MyTableVar AS m\nJOIN Employee ON m.EmployeeID = Employee.EmployeeID", + "previously_formatted": "SELECT x\nFROM @MyTableVar AS m\nJOIN Employee\n ON m.EmployeeID = Employee.EmployeeID" }, { "dialect": "tsql", @@ -5164,7 +5194,8 @@ { "dialect": "tsql", "sql": "SELECT (SELECT TOP 5 a, b FROM t ORDER BY b DESC FOR JSON PATH) AS j FROM x", - "formatted": "SELECT\n (\n SELECT TOP 5\n a,\n b\n FROM t\n ORDER BY\n b DESC\n FOR JSON\n PATH\n ) AS j\nFROM x" + "formatted": "SELECT\n (\n SELECT TOP 5\n a,\n b\n FROM t\n ORDER BY b DESC\n FOR JSON\n PATH\n ) AS j\nFROM x", + "previously_formatted": "SELECT\n (\n SELECT TOP 5\n a,\n b\n FROM t\n ORDER BY\n b DESC\n FOR JSON\n PATH\n ) AS j\nFROM x" }, { "dialect": "tsql", @@ -5274,7 +5305,8 @@ { "dialect": "tsql", "sql": "SELECT x FROM a INNER LOOP JOIN b ON b.id = a.id", - "formatted": "SELECT x\nFROM a\nINNER LOOP JOIN b\n ON b.id = a.id" + "formatted": "SELECT x\nFROM a\nINNER LOOP JOIN b ON b.id = a.id", + "previously_formatted": "SELECT x\nFROM a\nINNER LOOP JOIN b\n ON b.id = a.id" }, { "dialect": "tsql", diff --git a/crates/sqlquack-format/src/snowflake/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/snowflake/profile/tests/test_format_sql.rs index de507488c..a8b4145a4 100644 --- a/crates/sqlquack-format/src/snowflake/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/snowflake/profile/tests/test_format_sql.rs @@ -58,7 +58,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re sql: "select a from t join u on t.id=u.id", line_width: None, dialect: FormatDialect::Snowflake, - expected_outcome: Ok("SELECT a\nFROM t\nJOIN u\n ON t.id = u.id"), + expected_outcome: Ok("SELECT a\nFROM t\nJOIN u ON t.id = u.id"), }, test_types::FormatSqlTestCase { description: "two-character comparisons stay whole", @@ -183,9 +183,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re sql: "SELECT * FROM t PIVOT (SUM(v) FOR k IN (1+1, 2-1)) ORDER BY 1", line_width: None, dialect: FormatDialect::Snowflake, - expected_outcome: Ok( - "SELECT *\nFROM t PIVOT(SUM(v) FOR k IN (1+1, 2-1))\nORDER BY\n 1", - ), + expected_outcome: Ok("SELECT *\nFROM t PIVOT(SUM(v) FOR k IN (1+1, 2-1))\nORDER BY 1"), }, test_types::FormatSqlTestCase { description: "PIVOT IN concatenations keep their spacing", @@ -193,7 +191,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re line_width: None, dialect: FormatDialect::Snowflake, expected_outcome: Ok( - "SELECT *\nFROM t PIVOT(SUM(v) FOR k IN ('a'||'b', 'c'))\nORDER BY\n 1", + "SELECT *\nFROM t PIVOT(SUM(v) FOR k IN ('a'||'b', 'c'))\nORDER BY 1", ), }, test_types::FormatSqlTestCase { @@ -201,7 +199,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re sql: "SELECT * FROM t PIVOT (SUM(v) FOR k IN (- 1, 2)) ORDER BY 1", line_width: None, dialect: FormatDialect::Snowflake, - expected_outcome: Ok("SELECT *\nFROM t PIVOT(SUM(v) FOR k IN (- 1, 2))\nORDER BY\n 1"), + expected_outcome: Ok("SELECT *\nFROM t PIVOT(SUM(v) FOR k IN (- 1, 2))\nORDER BY 1"), }, test_types::FormatSqlTestCase { description: "select items without an alias keep their text, which names their column", diff --git a/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_corpus.rs b/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_corpus.rs index 45db2fe66..7d9ec53b5 100644 --- a/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_corpus.rs +++ b/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_corpus.rs @@ -22,17 +22,9 @@ fn given_dialect_corpus_when_formatting_then_output_layout_and_refusals_are_stab Departure { sql: "WITH t1 AS (SELECT 1 AS a), t2 AS (SELECT 1 AS a) SELECT COUNT(*) FROM (SELECT TOP 10 a FROM t1 UNION ALL SELECT TOP 10 a FROM t2 ORDER BY a DESC) AS t", formatted: Some( - "WITH t1 AS (\n SELECT 1 AS a\n),\n\nt2 AS (\n SELECT 1 AS a\n)\n\nSELECT COUNT(*)\nFROM (\n SELECT TOP 10 a\n FROM t1\n UNION ALL\n SELECT TOP 10 a\n FROM t2\n ORDER BY\n a DESC\n) AS t", + "WITH t1 AS (\n SELECT 1 AS a\n),\n\nt2 AS (\n SELECT 1 AS a\n)\n\nSELECT COUNT(*)\nFROM (\n SELECT TOP 10 a\n FROM t1\n UNION ALL\n SELECT TOP 10 a\n FROM t2\n ORDER BY a DESC\n) AS t", ), - reason: "A set operation's ORDER BY sits at the depth of its set operation with its items on the next line; SQLClean outdented it to the enclosing parenthesis and kept the items on its line", - receipt: None, - }, - Departure { - sql: "WITH t1 AS (SELECT 1 AS a), t2 AS (SELECT 1 AS a) SELECT TOP 10 a FROM t1 UNION ALL SELECT TOP 10 a FROM t2 ORDER BY a DESC", - formatted: Some( - "WITH t1 AS (\n SELECT 1 AS a\n),\n\nt2 AS (\n SELECT 1 AS a\n)\n\nSELECT TOP 10 a\nFROM t1\nUNION ALL\nSELECT TOP 10 a\nFROM t2\nORDER BY\n a DESC", - ), - reason: "A set operation's ORDER BY puts its items on the next line, as in every other dialect and SQL Server's own `ORDER BY\\n 1`; SQLClean kept them on the ORDER BY line", + reason: "A set operation's ORDER BY sits at the depth of its set operation; SQLClean outdented it to the enclosing parenthesis", receipt: None, }, Departure { diff --git a/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_sql.rs b/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_sql.rs index ce00d5ed3..5b5f6b042 100644 --- a/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_sql.rs +++ b/crates/sqlquack-format/src/sqlserver/profile/tests/test_format_sql.rs @@ -19,7 +19,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re sql: "SELECT * FROM #t JOIN ##g ON #t.id = ##g.id", line_width: None, dialect: FormatDialect::SqlServer, - expected_outcome: Ok("SELECT *\nFROM #t\nJOIN ##g\n ON #t.id = ##g.id"), + expected_outcome: Ok("SELECT *\nFROM #t\nJOIN ##g ON #t.id = ##g.id"), }, test_types::FormatSqlTestCase { description: "a SQL Server N prefix stays on its string, and a spaced N stays a column with an alias", @@ -234,7 +234,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re line_width: None, dialect: FormatDialect::SqlServer, expected_outcome: Ok( - "SELECT UPPER(a)\nFROM f(1) AS x, g(2) AS y\nJOIN h(3) AS z\n ON 1 = 1", + "SELECT UPPER(a)\nFROM f(1) AS x, g(2) AS y\nJOIN h(3) AS z ON 1 = 1", ), }, test_types::FormatSqlTestCase { @@ -365,9 +365,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re sql: "select x from a hash left outer hash join b on b.id = hash.id", line_width: None, dialect: FormatDialect::SqlServer, - expected_outcome: Ok( - "SELECT x\nFROM a hash\nLEFT OUTER hash JOIN b\n ON b.id = hash.id", - ), + expected_outcome: Ok("SELECT x\nFROM a hash\nLEFT OUTER hash JOIN b ON b.id = hash.id"), }, test_types::FormatSqlTestCase { description: "table hints stay with their table, an index or a FORCESEEK's columns in their own parentheses", @@ -375,7 +373,7 @@ fn given_sql_whose_spacing_carries_meaning_when_formatting_then_it_is_kept_or_re line_width: None, dialect: FormatDialect::SqlServer, expected_outcome: Ok( - "SELECT a\nFROM t WITH (nolock, FORCESEEK(ix_t(a, b)))\nJOIN u WITH (INDEX = 2, spatial_window_max_cells = 512)\n ON 1 = 1", + "SELECT a\nFROM t WITH (nolock, FORCESEEK(ix_t(a, b)))\nJOIN u WITH (INDEX = 2, spatial_window_max_cells = 512) ON 1 = 1", ), }, test_types::FormatSqlTestCase { diff --git a/website/src/content/docs/docs/cli/convert.mdx b/website/src/content/docs/docs/cli/convert.mdx index 4241e3c72..8f190afff 100644 --- a/website/src/content/docs/docs/cli/convert.mdx +++ b/website/src/content/docs/docs/cli/convert.mdx @@ -32,8 +32,7 @@ SELECT CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY - "ORDERS"."CUSTOMER_ID" +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/index.mdx b/website/src/content/docs/docs/index.mdx index 2c1011a58..a93ae1264 100644 --- a/website/src/content/docs/docs/index.mdx +++ b/website/src/content/docs/docs/index.mdx @@ -33,8 +33,7 @@ SELECT CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY - "ORDERS"."CUSTOMER_ID" +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/project.mdx b/website/src/content/docs/docs/project.mdx index ab99247ff..4acfad2c6 100644 --- a/website/src/content/docs/docs/project.mdx +++ b/website/src/content/docs/docs/project.mdx @@ -30,8 +30,7 @@ SELECT "ORDERS"."STATUS" AS "STATUS", CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" -GROUP BY - "ORDERS"."STATUS" +GROUP BY "ORDERS"."STATUS" sqlquack convert: done, converted ``` diff --git a/website/src/content/docs/docs/quickstart.mdx b/website/src/content/docs/docs/quickstart.mdx index fee7bcc9b..1454554d0 100644 --- a/website/src/content/docs/docs/quickstart.mdx +++ b/website/src/content/docs/docs/quickstart.mdx @@ -97,8 +97,7 @@ SELECT CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY - "ORDERS"."CUSTOMER_ID" +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted ``` From 9a6af3343d43db6f6ed47442e2207079cb109bea Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 05:48:12 +0000 Subject: [PATCH 7/8] docs(website): show the landing page's GROUP BY on its header line, as convert prints it The formatter keeps a single-item BY clause on its header line, so `sqlquack convert revenue.sql` prints GROUP BY and its column on one line. The docs pages already show that; the landing page still showed the column on the next line. --- website/public/index.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/website/public/index.html b/website/public/index.html index 24efa998c..e6dafb945 100644 --- a/website/public/index.html +++ b/website/public/index.html @@ -91,8 +91,7 @@

Run warehouse SQL locally.

CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24, 2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') -GROUP BY - "ORDERS"."CUSTOMER_ID" +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted From 0f0c43d78b66bc8391310408db4a71d0e1cf37d1 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 06:12:22 +0000 Subject: [PATCH 8/8] build(website): lock the playground's sqlquack-format at the workspace version The playground's lockfile recorded sqlquack-format at 0.4.1, the version when the dependency was added. The workspace is now 0.5.1, so the website build's --locked cargo commands refused to update it. --- website/wasm/Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/wasm/Cargo.lock b/website/wasm/Cargo.lock index 9fa34c46b..b14ec0249 100644 --- a/website/wasm/Cargo.lock +++ b/website/wasm/Cargo.lock @@ -262,7 +262,7 @@ dependencies = [ [[package]] name = "sqlquack-format" -version = "0.4.1" +version = "0.5.1" dependencies = [ "serde_json", "sqlquack-parse",