From c7f1288ea529a544bd7ce32844116be0f3fa67cf Mon Sep 17 00:00:00 2001 From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:50:10 -0700 Subject: [PATCH 1/2] Expand DuckDB ORDER BY ALL to positional ORDER BY for other targets --- crates/polyglot-sql/src/dialects/mod.rs | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/crates/polyglot-sql/src/dialects/mod.rs b/crates/polyglot-sql/src/dialects/mod.rs index a3e689f1..6be8b3b4 100644 --- a/crates/polyglot-sql/src/dialects/mod.rs +++ b/crates/polyglot-sql/src/dialects/mod.rs @@ -648,6 +648,49 @@ fn is_default_presto_date_format(fmt: &str) -> bool { fmt == "%Y-%m-%d" || fmt == "%F" } +/// Whether `e` is a lone, unquoted, unqualified `ALL` — how DuckDB's +/// `ORDER BY ALL` keyword is parsed (as a column/identifier/var, not a keyword). +#[cfg(feature = "transpile")] +fn is_order_by_all_marker(e: &Expression) -> bool { + match e { + Expression::Column(c) => { + c.table.is_none() && !c.name.quoted && c.name.name.eq_ignore_ascii_case("all") + } + Expression::Identifier(i) => !i.quoted && i.name.eq_ignore_ascii_case("all"), + Expression::Var(v) => v.this.eq_ignore_ascii_case("all"), + _ => false, + } +} + +/// Expand DuckDB `ORDER BY ALL` into positional `ORDER BY 1..n` over the +/// projection list (universally supported), preserving the sort direction. +/// Left untouched when the projection has a star — the column count is unknown. +#[cfg(feature = "transpile")] +fn expand_duckdb_order_by_all( + mut sel: Box, +) -> Box { + use crate::expressions::{Expression as E, Ordered}; + let matches = sel.order_by.as_ref().is_some_and(|ob| { + ob.expressions.len() == 1 && is_order_by_all_marker(&ob.expressions[0].this) + }); + let expandable = + !sel.expressions.is_empty() && !sel.expressions.iter().any(|e| matches!(e, E::Star(_))); + if matches && expandable { + let n = sel.expressions.len() as i64; + let ob = sel.order_by.as_mut().unwrap(); + let (desc, nulls_first) = (ob.expressions[0].desc, ob.expressions[0].nulls_first); + ob.expressions = (1..=n) + .map(|i| { + let mut o = Ordered::asc(E::number(i)); + o.desc = desc; + o.nulls_first = nulls_first; + o + }) + .collect(); + } + sel +} + /// Applies a dialect transform bottom-up through selected syntax children. /// /// The public entrypoint uses an explicit task stack for the recursion-heavy shapes @@ -3525,6 +3568,11 @@ impl Dialect { Ok(Expression::DataType(DT::Text)) } Expression::DataType(DT::Char { .. }) => Ok(Expression::DataType(DT::Text)), + // DuckDB `ORDER BY ALL` -> positional `ORDER BY 1..n` for + // targets without native `ORDER BY ALL` support. + Expression::Select(sel) if target != DialectType::DuckDB => { + Ok(Expression::Select(expand_duckdb_order_by_all(sel))) + } _ => Ok(e), })? } else { From 38a01e5ce3db8531dcee74776fe72d1e7cf66e2e Mon Sep 17 00:00:00 2001 From: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com> Date: Sat, 26 Sep 2026 00:02:13 -0700 Subject: [PATCH 2/2] ORDER BY ALL: reject * projections, keep ALL unquoted for DuckDB, add tests --- crates/polyglot-sql/src/dialects/mod.rs | 23 +++-- crates/polyglot-sql/src/generator.rs | 2 + .../polyglot-sql/tests/duckdb_order_by_all.rs | 88 +++++++++++++++++++ 3 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 crates/polyglot-sql/tests/duckdb_order_by_all.rs diff --git a/crates/polyglot-sql/src/dialects/mod.rs b/crates/polyglot-sql/src/dialects/mod.rs index 6be8b3b4..bbeb4581 100644 --- a/crates/polyglot-sql/src/dialects/mod.rs +++ b/crates/polyglot-sql/src/dialects/mod.rs @@ -664,18 +664,27 @@ fn is_order_by_all_marker(e: &Expression) -> bool { /// Expand DuckDB `ORDER BY ALL` into positional `ORDER BY 1..n` over the /// projection list (universally supported), preserving the sort direction. -/// Left untouched when the projection has a star — the column count is unknown. +/// A `*` projection has no known column count, so it is reported as an +/// unsupported translation rather than emitted as an `"ALL"` column reference. #[cfg(feature = "transpile")] fn expand_duckdb_order_by_all( mut sel: Box, -) -> Box { + target: DialectType, +) -> Result> { use crate::expressions::{Expression as E, Ordered}; let matches = sel.order_by.as_ref().is_some_and(|ob| { ob.expressions.len() == 1 && is_order_by_all_marker(&ob.expressions[0].this) }); - let expandable = - !sel.expressions.is_empty() && !sel.expressions.iter().any(|e| matches!(e, E::Star(_))); - if matches && expandable { + if !matches { + return Ok(sel); + } + if sel.expressions.is_empty() || sel.expressions.iter().any(|e| matches!(e, E::Star(_))) { + return Err(crate::error::Error::unsupported( + "ORDER BY ALL over a * projection cannot be expanded to positional ORDER BY", + target.to_string(), + )); + } + { let n = sel.expressions.len() as i64; let ob = sel.order_by.as_mut().unwrap(); let (desc, nulls_first) = (ob.expressions[0].desc, ob.expressions[0].nulls_first); @@ -688,7 +697,7 @@ fn expand_duckdb_order_by_all( }) .collect(); } - sel + Ok(sel) } /// Applies a dialect transform bottom-up through selected syntax children. @@ -3571,7 +3580,7 @@ impl Dialect { // DuckDB `ORDER BY ALL` -> positional `ORDER BY 1..n` for // targets without native `ORDER BY ALL` support. Expression::Select(sel) if target != DialectType::DuckDB => { - Ok(Expression::Select(expand_duckdb_order_by_all(sel))) + expand_duckdb_order_by_all(sel, target).map(Expression::Select) } _ => Ok(e), })? diff --git a/crates/polyglot-sql/src/generator.rs b/crates/polyglot-sql/src/generator.rs index cc5cf7db..9d6582d4 100644 --- a/crates/polyglot-sql/src/generator.rs +++ b/crates/polyglot-sql/src/generator.rs @@ -1501,6 +1501,8 @@ mod reserved_keywords { set.remove("range"); set.remove("row"); set.remove("values"); + // ORDER BY ALL needs ALL unquoted (inherited from POSTGRES_RESERVED) + set.remove("all"); set }); diff --git a/crates/polyglot-sql/tests/duckdb_order_by_all.rs b/crates/polyglot-sql/tests/duckdb_order_by_all.rs new file mode 100644 index 00000000..302a97ca --- /dev/null +++ b/crates/polyglot-sql/tests/duckdb_order_by_all.rs @@ -0,0 +1,88 @@ +//! DuckDB `ORDER BY ALL`: round-trips unquoted for DuckDB, expands to +//! positional `ORDER BY 1..n` for targets without it, and is rejected when the +//! projection is `*` (the column count is unknown). +use polyglot_sql::{transpile, DialectType}; + +#[test] +fn order_by_all_round_trips_unquoted_in_duckdb() { + let out = transpile( + "SELECT a, b FROM t ORDER BY ALL", + DialectType::DuckDB, + DialectType::DuckDB, + ) + .unwrap(); + assert_eq!(out, vec!["SELECT a, b FROM t ORDER BY ALL"]); +} + +#[test] +fn order_by_all_expands_to_positional_for_other_targets() { + for target in [ + DialectType::PostgreSQL, + DialectType::Snowflake, + DialectType::DataFusion, + ] { + let out = transpile( + "SELECT a, b FROM t ORDER BY ALL", + DialectType::DuckDB, + target, + ) + .unwrap(); + assert_eq!( + out, + vec!["SELECT a, b FROM t ORDER BY 1, 2"], + "target {target:?}" + ); + } +} + +#[test] +fn order_by_all_expansion_keeps_the_direction() { + // DuckDB sorts NULLs last for DESC too, whereas PostgreSQL defaults DESC to + // NULLS FIRST, so the expansion carries the explicit null order across. + let out = transpile( + "SELECT a, b FROM t ORDER BY ALL DESC", + DialectType::DuckDB, + DialectType::PostgreSQL, + ) + .unwrap(); + assert_eq!( + out, + vec!["SELECT a, b FROM t ORDER BY 1 DESC NULLS LAST, 2 DESC NULLS LAST"] + ); +} + +#[test] +fn order_by_all_expands_inside_subqueries() { + let out = transpile( + "SELECT a FROM t WHERE a IN (SELECT x, y FROM u ORDER BY ALL LIMIT 1)", + DialectType::DuckDB, + DialectType::PostgreSQL, + ) + .unwrap(); + assert_eq!( + out, + vec!["SELECT a FROM t WHERE a IN (SELECT x, y FROM u ORDER BY 1, 2 LIMIT 1)"] + ); +} + +#[test] +fn order_by_all_over_star_is_unsupported_for_other_targets() { + let err = transpile( + "SELECT * FROM t ORDER BY ALL", + DialectType::DuckDB, + DialectType::PostgreSQL, + ) + .unwrap_err(); + assert!(err.to_string().contains("ORDER BY ALL"), "got: {err}"); +} + +#[test] +fn a_quoted_column_named_all_is_not_expanded() { + let out = transpile( + "SELECT a, b FROM t ORDER BY \"all\"", + DialectType::DuckDB, + DialectType::PostgreSQL, + ) + .unwrap(); + assert_eq!(out, vec!["SELECT a, b FROM t ORDER BY \"all\""]); +}