From 0c8fbb5c781580cea261dc21e6817052eadd9486 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:28:50 +0000 Subject: [PATCH 1/3] playground: keep translated SQL's line layout and scroll it sideways The Translate stage's SQL block uses white-space: pre with horizontal scroll, so a clause-per-line layout shows as laid out. Diagnostics keep wrapping. A stage is a grid item, so it takes min-width: 0; without it, a long line widened the whole page. --- website/public/playground/playground.js | 3 ++- website/public/styles.css | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/website/public/playground/playground.js b/website/public/playground/playground.js index dc23822a1..c328cb7f8 100644 --- a/website/public/playground/playground.js +++ b/website/public/playground/playground.js @@ -191,7 +191,8 @@ async function run() { spans.push({ kind: refused ? 'warn' : 'error', span: diagnostic.span, text: tone(rendered(diagnostic, chars)) }); let body = ''; if (translate.status !== 'skipped') { - body = (translate.statements ?? []).map((s) => terminal(escape(s.sql))).join(''); + // Translated SQL keeps its line layout and scrolls sideways; diagnostics still wrap. + body = (translate.statements ?? []).map((s) => `
${escape(s.sql)}
`).join(''); body += (translate.diagnostics ?? []).map((d) => terminal(tone(rendered(d, chars)))).join(''); } const policy = translate.profile ? `policy: ${translate.profile}` : ''; diff --git a/website/public/styles.css b/website/public/styles.css index 0729c64ca..2bd7b4d25 100644 --- a/website/public/styles.css +++ b/website/public/styles.css @@ -1264,6 +1264,8 @@ footer a { padding: 0; border: 1px solid var(--inner-line); background: var(--panel); + /* A grid item: lets the translated SQL scroll inside the stage instead of widening the page. */ + min-width: 0; } .pg-stage header { @@ -1373,6 +1375,12 @@ footer a { overflow-wrap: anywhere; } +.pg-body .terminal.sql { + white-space: pre; + overflow-wrap: normal; + overflow-x: auto; +} + .pg-body .terminal + .terminal { border-top-style: dashed; } From 4ad0cb00cbd6faa1f410b6fd5f152b9eaaaf14bc Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:18:27 +0000 Subject: [PATCH 2/3] feat: lay converted DuckDB SQL out one clause per line Converted statements print each clause on its own line, select items and common table expressions one per line, queries indented by depth and long CASE expressions broken at WHEN, ELSE and END. Short ones stay on one line. Only whitespace between tokens changes. convert --compact keeps the single line, in --json too. Verify replays the laid-out SQL beside the single line and fails a case whose results differ, whose tokens differ, or whose layout is not idempotent. --- .../src/layout/_helpers/breaks.rs | 246 ++++++++++++++++++ .../src/layout/_helpers/mod.rs | 5 + .../src/layout/_helpers/plan.rs | 83 ++++++ .../src/layout/_helpers/tokens.rs | 118 +++++++++ .../sqlquack-duckdb/src/layout/constants.rs | 48 ++++ .../src/layout/main/checked_layout.rs | 26 ++ .../src/layout/main/layout_sql.rs | 19 ++ crates/sqlquack-duckdb/src/layout/main/mod.rs | 4 + crates/sqlquack-duckdb/src/layout/mod.rs | 9 + crates/sqlquack-duckdb/src/layout/models.rs | 59 +++++ crates/sqlquack-duckdb/src/layout/tests.rs | 4 + .../src/layout/tests/test_layout_sql.rs | 74 ++++++ .../src/layout/tests/test_types.rs | 6 + crates/sqlquack-duckdb/src/lib.rs | 1 + .../src/verify/_helpers/judge.rs | 11 + .../src/verify/_helpers/layout_check.rs | 53 ++++ .../src/verify/_helpers/mod.rs | 1 + crates/sqlquack-evidence/src/verify/types.rs | 5 + .../src/bindings/_helpers/convert_json.rs | 1 + crates/sqlquack-python/src/bindings/models.rs | 2 + crates/sqlquack/src/analysis/models.rs | 5 + .../src/cli/_helpers/convert_command.rs | 10 +- .../sqlquack/src/cli/main/convert_report.rs | 2 +- crates/sqlquack/src/cli/models.rs | 5 +- crates/sqlquack/tests/cli/test_convert.rs | 39 ++- crates/sqlquack/tests/cli/test_types.rs | 7 + python/src/sqlquack/api/main/convert.py | 1 + python/src/sqlquack/api/models.py | 1 + .../src/sqlquack/api/main/test_convert.py | 7 + website/src/content/docs/docs/cli/convert.mdx | 16 +- website/src/content/docs/docs/python.mdx | 4 +- website/wasm/src/lib.rs | 3 +- 32 files changed, 860 insertions(+), 15 deletions(-) create mode 100644 crates/sqlquack-duckdb/src/layout/_helpers/breaks.rs create mode 100644 crates/sqlquack-duckdb/src/layout/_helpers/mod.rs create mode 100644 crates/sqlquack-duckdb/src/layout/_helpers/plan.rs create mode 100644 crates/sqlquack-duckdb/src/layout/_helpers/tokens.rs create mode 100644 crates/sqlquack-duckdb/src/layout/constants.rs create mode 100644 crates/sqlquack-duckdb/src/layout/main/checked_layout.rs create mode 100644 crates/sqlquack-duckdb/src/layout/main/layout_sql.rs create mode 100644 crates/sqlquack-duckdb/src/layout/main/mod.rs create mode 100644 crates/sqlquack-duckdb/src/layout/mod.rs create mode 100644 crates/sqlquack-duckdb/src/layout/models.rs create mode 100644 crates/sqlquack-duckdb/src/layout/tests.rs create mode 100644 crates/sqlquack-duckdb/src/layout/tests/test_layout_sql.rs create mode 100644 crates/sqlquack-duckdb/src/layout/tests/test_types.rs create mode 100644 crates/sqlquack-evidence/src/verify/_helpers/layout_check.rs diff --git a/crates/sqlquack-duckdb/src/layout/_helpers/breaks.rs b/crates/sqlquack-duckdb/src/layout/_helpers/breaks.rs new file mode 100644 index 000000000..179072238 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/_helpers/breaks.rs @@ -0,0 +1,246 @@ +//! 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}; +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}; + +/// The walk over a statement, which breaks its lines when `broken`. +pub(crate) 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-duckdb/src/layout/_helpers/mod.rs b/crates/sqlquack-duckdb/src/layout/_helpers/mod.rs new file mode 100644 index 000000000..ad51c813c --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/_helpers/mod.rs @@ -0,0 +1,5 @@ +//! 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-duckdb/src/layout/_helpers/plan.rs b/crates/sqlquack-duckdb/src/layout/_helpers/plan.rs new file mode 100644 index 000000000..4ae876b27 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/_helpers/plan.rs @@ -0,0 +1,83 @@ +//! 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-duckdb/src/layout/_helpers/tokens.rs b/crates/sqlquack-duckdb/src/layout/_helpers/tokens.rs new file mode 100644 index 000000000..7e5235891 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/_helpers/tokens.rs @@ -0,0 +1,118 @@ +//! 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 +} + +/// 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-duckdb/src/layout/constants.rs b/crates/sqlquack-duckdb/src/layout/constants.rs new file mode 100644 index 000000000..110d453a2 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/constants.rs @@ -0,0 +1,48 @@ +//! The layout's indentation, width and clause words. + +/// One level of indentation. +pub const INDENT: &str = " "; + +/// 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"]; diff --git a/crates/sqlquack-duckdb/src/layout/main/checked_layout.rs b/crates/sqlquack-duckdb/src/layout/main/checked_layout.rs new file mode 100644 index 000000000..74892ef0c --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/main/checked_layout.rs @@ -0,0 +1,26 @@ +//! `checked_layout`: the layout of DuckDB SQL, checked to change only whitespace. + +use crate::layout::_helpers::tokens::tokens; +use crate::layout::main::layout_sql::layout_sql; + +/// `sql` laid out by [`layout_sql`], or why that changed more than whitespace or is not idempotent. +pub fn checked_layout(sql: &str) -> Result { + let laid_out = layout_sql(sql); + let texts = |sql: &str| -> Vec { + tokens(sql) + .iter() + .map(|token| token.text.to_owned()) + .collect() + }; + if texts(sql) != texts(&laid_out) { + return Err("its tokens differ from the single-line SQL's".to_owned()); + } + 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 layout_sql(&laid_out) != laid_out { + return Err("laying it out again changes it".to_owned()); + } + Ok(laid_out) +} diff --git a/crates/sqlquack-duckdb/src/layout/main/layout_sql.rs b/crates/sqlquack-duckdb/src/layout/main/layout_sql.rs new file mode 100644 index 000000000..81a8b5a7d --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/main/layout_sql.rs @@ -0,0 +1,19 @@ +//! `layout_sql`: DuckDB SQL laid out over lines, with only the whitespace between tokens changed. + +use crate::layout::_helpers::breaks::start; +use crate::layout::_helpers::plan::Plan; +use crate::layout::_helpers::tokens::{Token, tokens}; +use crate::layout::constants::INLINE_WIDTH; + +/// `sql` one clause and select item per line, nested queries and long `CASE`s indented by depth. +pub fn layout_sql(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 +} diff --git a/crates/sqlquack-duckdb/src/layout/main/mod.rs b/crates/sqlquack-duckdb/src/layout/main/mod.rs new file mode 100644 index 000000000..78c00d85f --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/main/mod.rs @@ -0,0 +1,4 @@ +//! One building block per module. + +pub mod checked_layout; +pub mod layout_sql; diff --git a/crates/sqlquack-duckdb/src/layout/mod.rs b/crates/sqlquack-duckdb/src/layout/mod.rs new file mode 100644 index 000000000..062f5bfcc --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/mod.rs @@ -0,0 +1,9 @@ +//! A simple layout of DuckDB SQL over lines, changing only the whitespace between tokens. + +pub(crate) mod _helpers; +pub mod constants; +pub mod main; +pub(crate) mod models; + +#[cfg(test)] +mod tests; diff --git a/crates/sqlquack-duckdb/src/layout/models.rs b/crates/sqlquack-duckdb/src/layout/models.rs new file mode 100644 index 000000000..69e1e00b9 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/models.rs @@ -0,0 +1,59 @@ +//! 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-duckdb/src/layout/tests.rs b/crates/sqlquack-duckdb/src/layout/tests.rs new file mode 100644 index 000000000..e08edccf2 --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/tests.rs @@ -0,0 +1,4 @@ +#[path = "tests/test_layout_sql.rs"] +mod test_layout_sql; +#[path = "tests/test_types.rs"] +mod test_types; diff --git a/crates/sqlquack-duckdb/src/layout/tests/test_layout_sql.rs b/crates/sqlquack-duckdb/src/layout/tests/test_layout_sql.rs new file mode 100644 index 000000000..41499469b --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/tests/test_layout_sql.rs @@ -0,0 +1,74 @@ +use crate::layout::main::checked_layout::checked_layout; +use crate::layout::tests::test_types::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 + ); + } +} diff --git a/crates/sqlquack-duckdb/src/layout/tests/test_types.rs b/crates/sqlquack-duckdb/src/layout/tests/test_types.rs new file mode 100644 index 000000000..820149f5d --- /dev/null +++ b/crates/sqlquack-duckdb/src/layout/tests/test_types.rs @@ -0,0 +1,6 @@ +/// 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, +} diff --git a/crates/sqlquack-duckdb/src/lib.rs b/crates/sqlquack-duckdb/src/lib.rs index bbd8debec..e8dcfc7e3 100644 --- a/crates/sqlquack-duckdb/src/lib.rs +++ b/crates/sqlquack-duckdb/src/lib.rs @@ -9,6 +9,7 @@ pub mod checking; pub mod floats; pub mod hashing; pub mod held; +pub mod layout; pub mod raises; pub mod semi_structured; pub mod temporal; diff --git a/crates/sqlquack-evidence/src/verify/_helpers/judge.rs b/crates/sqlquack-evidence/src/verify/_helpers/judge.rs index 71ab602b4..59ff087a9 100644 --- a/crates/sqlquack-evidence/src/verify/_helpers/judge.rs +++ b/crates/sqlquack-evidence/src/verify/_helpers/judge.rs @@ -18,6 +18,7 @@ use crate::results::main::load_result::load_result; use crate::results::models::{RecordedOutcome, SessionDefaults, StoredResult}; use crate::verify::_helpers::compile_errors::undeclared_compile_error; use crate::verify::_helpers::error_check::error_check; +use crate::verify::_helpers::layout_check::{layout_failure, reads_clock}; use crate::verify::_helpers::microsecond_evidence::differs_below_microseconds; use crate::verify::_helpers::row_checks::{RowCheck, row_check}; use crate::verify::_helpers::session::{SessionCheck, session_check}; @@ -124,6 +125,12 @@ fn judge_stored( null_order: policy.comparator.null_order.duckdb_setting().to_owned(), }; let replayed = replay_on_duckdb((&setup, &duckdb_sql), python.as_deref(), &settings); + let layout = layout_failure( + &converter.laid_out(&duckdb_sql), + (&setup, &duckdb_sql), + (&replayed, reads_clock(provenance.divergences.keys())), + (python.as_deref(), &settings), + ); let duckdb_error = replayed.as_ref().err().cloned(); let verdict = match &case.expected_divergence { Some(divergence) => divergence_verdict( @@ -157,6 +164,10 @@ fn judge_stored( }, (verdict, _) => verdict, }; + let verdict = match layout { + Some(reason) => Verdict::Failed { reason }, + None => verdict, + }; let error_check = match (&verdict, &stored.metadata.error, duckdb_error) { (Verdict::Passed, Some(recorded), Some(error)) => Some(error_check( (recorded, &error), diff --git a/crates/sqlquack-evidence/src/verify/_helpers/layout_check.rs b/crates/sqlquack-evidence/src/verify/_helpers/layout_check.rs new file mode 100644 index 000000000..79ca8ee08 --- /dev/null +++ b/crates/sqlquack-evidence/src/verify/_helpers/layout_check.rs @@ -0,0 +1,53 @@ +//! The layout check: the laid-out SQL returns exactly what the single-line SQL returns. + +use sqlquack_compare::values::models::ResultSet; + +use crate::replay::main::replay_on_duckdb::replay_on_duckdb; +use crate::replay::models::{PythonReplay, ReplaySettings}; + +/// The divergence whose values read the clock, so two replays may differ by the time between them. +const CLOCK_DIVERGENCE: &str = "current_time"; + +/// Why the laid-out SQL returns another result or error than the single-line `sql`, if it does. +pub(crate) fn layout_failure( + laid_out: &Result, + (setup, sql): (&[String], &str), + (single_line, reads_clock): (&Result, bool), + (python, settings): (Option<&PythonReplay>, &ReplaySettings), +) -> Option { + let laid_out = match laid_out { + Ok(laid_out) if laid_out == sql => return None, + Ok(laid_out) => laid_out, + Err(reason) => return Some(format!("the layout changed more than whitespace: {reason}")), + }; + let replayed = replay_on_duckdb((setup, laid_out), python, settings); + let same = match (single_line, &replayed) { + (Ok(single_line), Ok(replayed)) => same_result(single_line, replayed, reads_clock), + (Err(single_line), Err(replayed)) => first_line(single_line) == first_line(replayed), + _ => false, + }; + (!same).then(|| { + format!( + "the laid-out SQL returned another result than the single-line SQL: {}", + match &replayed { + Ok(_) => "different rows or columns".to_owned(), + Err(error) => first_line(error).to_owned(), + } + ) + }) +} + +/// Whether the provenance's divergences include one whose values read the clock. +pub(crate) fn reads_clock<'name>(mut divergences: impl Iterator) -> bool { + divergences.any(|divergence| divergence == CLOCK_DIVERGENCE) +} + +/// Whether two results have the same schema and, unless they read the clock, the same rows. +fn same_result(left: &ResultSet, right: &ResultSet, reads_clock: bool) -> bool { + left.schema == right.schema && (reads_clock || left.batches == right.batches) +} + +/// An error's first line; later lines quote the SQL with its line numbers. +fn first_line(error: &str) -> &str { + error.lines().next().unwrap_or_default() +} diff --git a/crates/sqlquack-evidence/src/verify/_helpers/mod.rs b/crates/sqlquack-evidence/src/verify/_helpers/mod.rs index 99dc475bf..3913899e9 100644 --- a/crates/sqlquack-evidence/src/verify/_helpers/mod.rs +++ b/crates/sqlquack-evidence/src/verify/_helpers/mod.rs @@ -2,6 +2,7 @@ pub(crate) mod claims; pub(crate) mod compile_errors; pub(crate) mod error_check; pub(crate) mod judge; +pub(crate) mod layout_check; pub(crate) mod messages; pub(crate) mod microsecond_evidence; pub(crate) mod row_checks; diff --git a/crates/sqlquack-evidence/src/verify/types.rs b/crates/sqlquack-evidence/src/verify/types.rs index 32ebb1dd1..2c90fd54f 100644 --- a/crates/sqlquack-evidence/src/verify/types.rs +++ b/crates/sqlquack-evidence/src/verify/types.rs @@ -11,6 +11,11 @@ use crate::verify::models::Conversion; pub trait CaseConverter: std::fmt::Debug { /// The conversion of `case`. fn convert(&self, case: &LoadedCase) -> Conversion; + + /// The converted `sql` as users see it, or why laying it out changed more than whitespace. + fn laid_out(&self, sql: &str) -> Result { + Ok(sql.to_owned()) + } } /// For each rule id, the failure modes it declares. diff --git a/crates/sqlquack-python/src/bindings/_helpers/convert_json.rs b/crates/sqlquack-python/src/bindings/_helpers/convert_json.rs index b265a356f..f3cecd24b 100644 --- a/crates/sqlquack-python/src/bindings/_helpers/convert_json.rs +++ b/crates/sqlquack-python/src/bindings/_helpers/convert_json.rs @@ -21,6 +21,7 @@ pub(crate) fn convert_json(sql: &str, request: &str, path: &str) -> PyResult, /// `--session NAME=VALUE`, by name. pub session: BTreeMap, + /// `--compact`. + pub compact: bool, } diff --git a/crates/sqlquack/src/analysis/models.rs b/crates/sqlquack/src/analysis/models.rs index d0e302722..64982c0ae 100644 --- a/crates/sqlquack/src/analysis/models.rs +++ b/crates/sqlquack/src/analysis/models.rs @@ -10,6 +10,7 @@ use sqlquack_core::parsing::models::SourceDialect; use sqlquack_core::provenance::main::merge_provenance::merge_provenance; use sqlquack_core::provenance::models::Provenance; use sqlquack_core::refusals::models::Refusal; +use sqlquack_duckdb::layout::main::checked_layout::checked_layout; use sqlquack_evidence::cases::models::LoadedCase; use sqlquack_evidence::constants::{INPUT_RELATION, ORDINAL_COLUMN}; use sqlquack_evidence::replay::models::{PythonFunction, PythonReplay}; @@ -156,6 +157,10 @@ impl CaseConverter for PipelineConverter { }, } } + + fn laid_out(&self, sql: &str) -> Result { + checked_layout(sql) + } } /// A Python function as SQLQuack's runtime registers it in DuckDB. diff --git a/crates/sqlquack/src/cli/_helpers/convert_command.rs b/crates/sqlquack/src/cli/_helpers/convert_command.rs index 853e50caf..e45c23f31 100644 --- a/crates/sqlquack/src/cli/_helpers/convert_command.rs +++ b/crates/sqlquack/src/cli/_helpers/convert_command.rs @@ -12,6 +12,7 @@ use sqlquack_core::diagnostics::main::unplaced_diagnostic::unplaced_diagnostic; use sqlquack_core::diagnostics::models::Diagnostic; use sqlquack_core::provenance::main::merge_provenance::merge_provenance; use sqlquack_core::refusals::models::Refusal; +use sqlquack_duckdb::layout::main::layout_sql::layout_sql; use sqlquack_policy::policy::constants::{SESSION_TIME_ZONE, TIMEZONE_PARAMETER}; use sqlquack_policy::policy::models::{DivergenceFailure, Policy}; use sqlquack_rules::conversion::models::ConversionOptions; @@ -253,10 +254,10 @@ pub(crate) fn chosen_options( ) } -/// The report of a finished analysis of `sql`. +/// The report of a finished analysis of `sql`, its SQL one clause per line unless `compact`. pub(crate) fn analysis_report( analysis: &Analysis, - sql: &str, + (sql, compact): (&str, bool), (schema, options, policy): (&Schema, &ConversionOptions, Option<&Policy>), (arguments, parameters, profile): (&PolicyArguments, &BTreeMap, String), ) -> Result { @@ -292,7 +293,10 @@ pub(crate) fn analysis_report( true => analysis .conversions .iter() - .map(|done| done.sql.clone()) + .map(|done| match compact { + true => done.sql.clone(), + false => layout_sql(&done.sql), + }) .collect::>() .join(";\n"), false => String::new(), diff --git a/crates/sqlquack/src/cli/main/convert_report.rs b/crates/sqlquack/src/cli/main/convert_report.rs index 374271984..ee046f233 100644 --- a/crates/sqlquack/src/cli/main/convert_report.rs +++ b/crates/sqlquack/src/cli/main/convert_report.rs @@ -46,7 +46,7 @@ pub fn convert_report(arguments: &ConvertArguments, sql: &str) -> Result, + /// Print each converted statement on one line instead of one clause per line, in `--json` too. + #[arg(long)] + pub compact: bool, /// How to print the report on standard output. #[command(flatten)] pub output: OutputArguments, @@ -369,7 +372,7 @@ pub struct ConvertReport { pub converted: bool, /// The session profile conversion ran under. pub profile: String, - /// The DuckDB SQL, one statement after another; empty when refused. + /// The DuckDB SQL, one clause per line unless `--compact`; empty when refused. pub sql: String, /// Each refusal. pub refusals: Vec, diff --git a/crates/sqlquack/tests/cli/test_convert.rs b/crates/sqlquack/tests/cli/test_convert.rs index 6f54b5aa9..fbc076b97 100644 --- a/crates/sqlquack/tests/cli/test_convert.rs +++ b/crates/sqlquack/tests/cli/test_convert.rs @@ -7,9 +7,9 @@ use crate::helpers::{ use crate::test_types::{ ColumnNameTestCase, ConvertDiagnosticTestCase, ConvertOutcome, ConvertTestCase, ConvertedRunTestCase, CountedCallsTestCase, CrossingTestCase, DedicatedOverrideTestCase, - DeepChainTestCase, DivergenceHelpTestCase, FailureKindTestCase, OutputFormatTestCase, - OverrideFailureTestCase, OverrideTestCase, PolicyFreeTestCase, PythonRefusalTestCase, - PythonRunTestCase, RunErrorTestCase, SchemaErrorTestCase, ValueTestCase, + DeepChainTestCase, DivergenceHelpTestCase, FailureKindTestCase, LayoutFlagsTestCase, + OutputFormatTestCase, OverrideFailureTestCase, OverrideTestCase, PolicyFreeTestCase, + PythonRefusalTestCase, PythonRunTestCase, RunErrorTestCase, SchemaErrorTestCase, ValueTestCase, }; use sha2::{Digest, Sha256}; @@ -1584,6 +1584,39 @@ fn given_refused_sql_when_converting_then_text_and_json_carry_the_same_coded_dia } } +#[test] +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", + 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)))", + }, + LayoutFlagsTestCase { + description: "--compact prints the statement on one line, in text and in --json", + flags: &["--compact"], + expected_sql: "SELECT CAST(1 AS DECIMAL(1,0)) AS \"A\", CAST(2 AS DECIMAL(1,0)) AS \"B\", \ + 'some longer text here' AS \"C\" WHERE (CAST(1 AS DECIMAL(1,0)) = CAST(1 AS DECIMAL(1,0)))", + }, + ]; + let sql = "SELECT 1 AS a, 2 AS b, 'some longer text here' AS c WHERE 1 = 1"; + for test_case in test_cases { + let (text, _) = convert_streams(sql, &[&["--builtin-policy"], test_case.flags].concat()); + let (json, _) = convert_streams( + sql, + &[&["--builtin-policy", "--json"], test_case.flags].concat(), + ); + let report: serde_json::Value = serde_json::from_str(&json).expect("a JSON report"); + assert_eq!( + (text.trim_end(), report["sql"].as_str()), + (test_case.expected_sql, Some(test_case.expected_sql)), + "{}", + test_case.description + ); + } +} + #[test] fn given_output_flags_when_running_then_format_json_and_json_agree_and_help_lists_exit_status() { let test_cases = [ diff --git a/crates/sqlquack/tests/cli/test_types.rs b/crates/sqlquack/tests/cli/test_types.rs index 191b9a7bf..1c9c6d32f 100644 --- a/crates/sqlquack/tests/cli/test_types.rs +++ b/crates/sqlquack/tests/cli/test_types.rs @@ -293,3 +293,10 @@ pub(crate) struct SchemaErrorTestCase { pub(crate) schema: &'static str, pub(crate) expected_error: &'static str, } + +/// Convert flags and the SQL convert prints under them, from standard output or `--json`'s `sql`. +pub(crate) struct LayoutFlagsTestCase { + pub(crate) description: &'static str, + pub(crate) flags: &'static [&'static str], + pub(crate) expected_sql: &'static str, +} diff --git a/python/src/sqlquack/api/main/convert.py b/python/src/sqlquack/api/main/convert.py index ee400b4e7..c01287c7d 100644 --- a/python/src/sqlquack/api/main/convert.py +++ b/python/src/sqlquack/api/main/convert.py @@ -28,6 +28,7 @@ def convert( "loud_divergences_only": chosen.loud_divergences_only, "overrides": path_argument(chosen.overrides), "verified_overrides": dict(chosen.verified_overrides), + "compact": chosen.compact, } result: dict[str, Any] = loaded(lambda: convert_json(sql, json.dumps(request), path)) report: dict[str, Any] = result["report"] diff --git a/python/src/sqlquack/api/models.py b/python/src/sqlquack/api/models.py index ef9d09ee8..e22727029 100644 --- a/python/src/sqlquack/api/models.py +++ b/python/src/sqlquack/api/models.py @@ -76,3 +76,4 @@ class ConvertOptions: loud_divergences_only: bool = False overrides: str | os.PathLike[str] | None = None verified_overrides: Mapping[str, str] = field(default_factory=dict) + compact: bool = False diff --git a/tests/unit/python/src/sqlquack/api/main/test_convert.py b/tests/unit/python/src/sqlquack/api/main/test_convert.py index 44c8a5df9..0e3c5115e 100644 --- a/tests/unit/python/src/sqlquack/api/main/test_convert.py +++ b/tests/unit/python/src/sqlquack/api/main/test_convert.py @@ -31,6 +31,13 @@ options=ConvertOptions(builtin_policy=True, schema="schema.json"), expected_converted=True, ), + CommandLineTestCase( + description="a converted query on one line", + sql="SELECT order_id, price * 2 AS doubled FROM orders", + arguments=("--builtin-policy", "--schema", "schema.json", "--compact"), + options=ConvertOptions(builtin_policy=True, schema="schema.json", compact=True), + expected_converted=True, + ), CommandLineTestCase( description="a query refused for an undeclared relation", sql="SELECT a FROM customers", diff --git a/website/src/content/docs/docs/cli/convert.mdx b/website/src/content/docs/docs/cli/convert.mdx index 36710f557..d726b0110 100644 --- a/website/src/content/docs/docs/cli/convert.mdx +++ b/website/src/content/docs/docs/cli/convert.mdx @@ -20,12 +20,18 @@ echo "SELECT 1 AS total" | sqlquack convert --builtin-policy --json ``` `PATH` is a SQL file. Without one, or with `-`, `convert` reads standard input. The DuckDB SQL goes -to standard output. Progress, the known divergences and assumptions it relies on, and refusals go -to standard error. +to standard output, laid out with each clause on its own line, select items one per line, and +nested queries and long `CASE` expressions indented; short statements stay on one line. Progress, +the known divergences and assumptions it relies on, and refusals go to standard error. ```console $ sqlquack convert revenue.sql --schema schema.json --builtin-policy -SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') GROUP BY "ORDERS"."CUSTOMER_ID" +SELECT + "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" +FROM "ORDERS" AS "ORDERS" +WHERE ("ORDERS"."STATUS" = 'shipped') +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted under the built-in default ``` @@ -56,6 +62,7 @@ SQL that does not parse, or that Snowflake rejects, is refused with the same cod | `--session NAME=VALUE` | A Snowflake session parameter of your session. Repeatable. See [Sessions](/docs/policy/#sessions). | | `--overrides ` | User overrides: a `FUNCTION()` file, or a directory of them, each with `replaces` and `dialect duckdb`. | | `--verified-override NAME=SHA256` | An override verified for the body with that hash, as `convert --json` reports. | +| `--compact` | Print each statement on one line instead, in the JSON report too. | | `--format text\|json`, `--json` | Print the report as text or JSON. | `convert` also takes the policy flags: `--policy`, `--builtin-policy`, `--strict`, @@ -82,7 +89,8 @@ With `--json`, `convert` prints one object: } ``` -A refusal in `refusals` carries the same [diagnostic fields](/docs/diagnostics/overview/#json-output). +`sql` is laid out as the text output prints it, or on one line with `--compact`. A refusal in +`refusals` carries the same [diagnostic fields](/docs/diagnostics/overview/#json-output). ## Exit status diff --git a/website/src/content/docs/docs/python.mdx b/website/src/content/docs/docs/python.mdx index 3af634699..636391ec4 100644 --- a/website/src/content/docs/docs/python.mdx +++ b/website/src/content/docs/docs/python.mdx @@ -75,8 +75,8 @@ SQL in rendered diagnostics. It returns a `CheckResult`: `ConvertOptions` has one field per [`convert`](/docs/cli/convert/) flag, with the same defaults: `schema`, `policy`, `builtin_policy`, `session`, `strict`, `utc_session`, -`confirm_declared_offsets`, `python_runtime`, `loud_divergences_only`, `overrides` and -`verified_overrides`. `schema`, `policy` and `overrides` take a path (`str` or `os.PathLike`), not +`confirm_declared_offsets`, `python_runtime`, `loud_divergences_only`, `overrides`, +`verified_overrides` and `compact`. `schema`, `policy` and `overrides` take a path (`str` or `os.PathLike`), not the parsed contents. `session` and `verified_overrides` take a mapping, such as `session={"TIMEZONE": "UTC"}`, not a list of `NAME=VALUE` strings. With `policy` and `builtin_policy` both unset, `convert` reads `sqlquack.toml` from the current directory and raises diff --git a/website/wasm/src/lib.rs b/website/wasm/src/lib.rs index ddc57a502..c1e429f72 100644 --- a/website/wasm/src/lib.rs +++ b/website/wasm/src/lib.rs @@ -23,6 +23,7 @@ use sqlquack_core::refusals::models::{Refusal, RefusalCategory}; use sqlquack_duckdb::blocks::main::column_type::column_type; use sqlquack_duckdb::blocks::main::quoted_identifier::quoted_identifier; use sqlquack_duckdb::checking::constants::SEMI_STRUCTURED_HOLDING_TYPE; +use sqlquack_duckdb::layout::main::layout_sql::layout_sql; use sqlquack_policy::policy::models::Policy; use sqlquack_rules::catalogue::main::builtin_catalogue::builtin_catalogue; use sqlquack_rules::conversion::main::convert_statement::convert_statement; @@ -306,7 +307,7 @@ fn report(sql: &str, schema: &str, settings: &str) -> Value { let elapsed = Duration::from_secs_f64((now_ms() - started).max(0.0) / 1000.0); match done.and_then(|done| within_budget(done, (sql, elapsed), &options)) { Ok(done) => converted.push(json!({ - "sql": done.sql, + "sql": layout_sql(&done.sql), "columns": done.columns.iter().map(|column| json!({ "name": column.name, "type": column.data_type.to_string(), From 642b014b96dde981ae2699390a1acd3016d23a21 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:08:00 +0000 Subject: [PATCH 3/3] feat(playground): lay out translated SQL unless compact, as convert does The playground engine takes a compact setting like convert --compact, and the docs pages that print convert output show the laid-out form. --- website/src/content/docs/docs/index.mdx | 7 +++++- website/src/content/docs/docs/quickstart.mdx | 7 +++++- website/tests/playground.test.mjs | 14 +++++++++++ website/wasm/src/lib.rs | 26 ++++++++++++++------ 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/website/src/content/docs/docs/index.mdx b/website/src/content/docs/docs/index.mdx index 419a16653..f43d8ab34 100644 --- a/website/src/content/docs/docs/index.mdx +++ b/website/src/content/docs/docs/index.mdx @@ -26,7 +26,12 @@ Once the schema declares `ORDERS`, the same file converts: ```console $ sqlquack convert revenue.sql --builtin-policy --schema schema.json -SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') GROUP BY "ORDERS"."CUSTOMER_ID" +SELECT + "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" +FROM "ORDERS" AS "ORDERS" +WHERE ("ORDERS"."STATUS" = 'shipped') +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted under the built-in default ``` diff --git a/website/src/content/docs/docs/quickstart.mdx b/website/src/content/docs/docs/quickstart.mdx index c647d0e30..7743e03e2 100644 --- a/website/src/content/docs/docs/quickstart.mdx +++ b/website/src/content/docs/docs/quickstart.mdx @@ -86,7 +86,12 @@ statement reads. Declare them in a [schema file](/docs/schema/): ```console $ sqlquack convert revenue.sql --schema schema.json --builtin-policy -SELECT "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" FROM "ORDERS" AS "ORDERS" WHERE ("ORDERS"."STATUS" = 'shipped') GROUP BY "ORDERS"."CUSTOMER_ID" +SELECT + "ORDERS"."CUSTOMER_ID" AS "CUSTOMER_ID", + CAST(sum("ORDERS"."TOTAL") AS DECIMAL(24,2)) AS "REVENUE" +FROM "ORDERS" AS "ORDERS" +WHERE ("ORDERS"."STATUS" = 'shipped') +GROUP BY "ORDERS"."CUSTOMER_ID" sqlquack convert: done, converted under the built-in default ``` diff --git a/website/tests/playground.test.mjs b/website/tests/playground.test.mjs index b115283f6..e3886e8d8 100644 --- a/website/tests/playground.test.mjs +++ b/website/tests/playground.test.mjs @@ -103,3 +103,17 @@ test('every example stops at the stage it is meant to, and the default translate assert.deepEqual(refused.diagnostics.map((d) => d.code), ['SQ0008']); assert.ok(CAPTIONS.refused); }); + +test('translated SQL is laid out one clause per line unless the compact setting asks for one line', () => { + const sql = "SELECT order_id, note, 'a longer literal' AS label FROM orders WHERE order_id > 1"; + const translated = (settings) => JSON.parse(analyze(sql, SCHEMA, JSON.stringify(settings))).translate.statements[0].sql; + const laidOut = translated({}); + 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)))', + ); + const compact = translated({ compact: true }); + assert.equal(compact, laidOut.replace(/\n */g, ' ')); + assert.ok(!compact.includes('\n')); +}); diff --git a/website/wasm/src/lib.rs b/website/wasm/src/lib.rs index c1e429f72..f6ee9c655 100644 --- a/website/wasm/src/lib.rs +++ b/website/wasm/src/lib.rs @@ -42,7 +42,8 @@ use sqlquack_syntax::{Dialect, Expression, ParseOptions}; use wasm_bindgen::prelude::wasm_bindgen; /// Every stage's result for `sql` over the relations `schema` declares, as JSON. `settings` is -/// JSON `{"policy": TOML, "session": "NAME=VALUE" lines, "strict": bool, "utc_session": bool}`; +/// JSON `{"policy": TOML, "session": "NAME=VALUE" lines, "strict": bool, "utc_session": bool, +/// "compact": bool}`, `compact` printing each statement on one line as `convert --compact` does; /// an empty policy is the built-in default, as `sqlquack convert --builtin-policy`. #[wasm_bindgen] pub fn analyze(sql: &str, schema: &str, settings: Option) -> String { @@ -128,15 +129,14 @@ struct Settings { session: String, strict: bool, utc_session: bool, + compact: bool, } /// Conversion options for `settings` as `sqlquack convert` chooses them, and the matched profile; /// a session no recorded profile covers is a refusal, as on the command line. -fn chosen_options(settings: &str) -> Result, String> { - let settings: Settings = match settings.trim() { - "" => Settings::default(), - text => serde_json::from_str(text).map_err(|error| error.to_string())?, - }; +fn chosen_options( + settings: &Settings, +) -> Result, String> { let policy = match settings.policy.trim() { "" => None, text => Some( @@ -209,7 +209,14 @@ fn report(sql: &str, schema: &str, settings: &str) -> Value { Ok(catalog) => catalog, Err(error) => return json!({ "schema_error": error }), }; - let session = match chosen_options(settings) { + let settings: Settings = match settings.trim() { + "" => Settings::default(), + text => match serde_json::from_str(text) { + Ok(settings) => settings, + Err(error) => return json!({ "settings_error": error.to_string() }), + }, + }; + let session = match chosen_options(&settings) { Ok(session) => session, Err(error) => return json!({ "settings_error": error }), }; @@ -307,7 +314,10 @@ fn report(sql: &str, schema: &str, settings: &str) -> Value { let elapsed = Duration::from_secs_f64((now_ms() - started).max(0.0) / 1000.0); match done.and_then(|done| within_budget(done, (sql, elapsed), &options)) { Ok(done) => converted.push(json!({ - "sql": layout_sql(&done.sql), + "sql": match settings.compact { + true => done.sql.clone(), + false => layout_sql(&done.sql), + }, "columns": done.columns.iter().map(|column| json!({ "name": column.name, "type": column.data_type.to_string(),