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-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 500505d5a..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 @@ -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", @@ -87,6 +96,73 @@ 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 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]", + 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..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. @@ -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)) } @@ -1301,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] @@ -1314,6 +1317,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 { @@ -1321,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/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..50eda7e3f 100644 --- a/crates/sqlquack-format/src/format/_helpers/printed_layout.rs +++ b/crates/sqlquack-format/src/format/_helpers/printed_layout.rs @@ -16,11 +16,27 @@ 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", ]; -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", @@ -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| { @@ -993,7 +1070,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 +1092,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 +1101,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/_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 f074906c5..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. @@ -175,7 +181,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 +191,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,9 +206,11 @@ 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] = [ + "=", "<>", "!=", "<", ">", "<=", ">=", "+", "-", "*", "/", "//", "%", "||", "->", ]; +/// 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. 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(), } 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/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..677314319 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,32 @@ 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(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, + }; + 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..b2559da73 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -1874,3 +1874,79 @@ 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 = [ + 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 + ); + } +} 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..e6dafb945 100644 --- a/website/public/index.html +++ b/website/public/index.html @@ -88,7 +88,7 @@

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" @@ -159,10 +159,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 +311,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..8f190afff 100644 --- a/website/src/content/docs/docs/cli/convert.mdx +++ b/website/src/content/docs/docs/cli/convert.mdx @@ -29,7 +29,7 @@ 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" @@ -78,7 +78,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..a93ae1264 100644 --- a/website/src/content/docs/docs/index.mdx +++ b/website/src/content/docs/docs/index.mdx @@ -30,7 +30,7 @@ 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" 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..4acfad2c6 100644 --- a/website/src/content/docs/docs/project.mdx +++ b/website/src/content/docs/docs/project.mdx @@ -28,7 +28,7 @@ $ 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" 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..1454554d0 100644 --- a/website/src/content/docs/docs/quickstart.mdx +++ b/website/src/content/docs/docs/quickstart.mdx @@ -94,7 +94,7 @@ 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" 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..b14ec0249 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.5.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)), }