From 730df956d5095ab9858a7dcf4b2f870e17434add Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:16:30 +0000 Subject: [PATCH 1/4] fix(semantics): accept LAG and LEAD with one or two arguments in DuckDB duckdb_functions() lists each window function once with every parameter, so the catalogue recorded only three-argument LAG and LEAD, and arity checks refused LAG(x), LAG(x, n), LEAD(x) and LEAD(x, n), which DuckDB runs. The extractor now asks the engine which shorter counts each fixed-arity window function binds and records one row per count. Every other window function already matched DuckDB 2.0.0. --- .../_helpers/duckdb_catalog/functions.csv | 4 ++ .../profile/tests/test_statement_findings.rs | 48 +++++++++++++++++++ scripts/duckdb_catalogue/_helpers/engine.py | 31 +++++++++++- scripts/duckdb_catalogue/constants.py | 2 + scripts/duckdb_catalogue/main/extract.py | 16 +++++-- 5 files changed, 94 insertions(+), 7 deletions(-) diff --git a/crates/sqlquack-semantics/src/duckdb/profile/_helpers/duckdb_catalog/functions.csv b/crates/sqlquack-semantics/src/duckdb/profile/_helpers/duckdb_catalog/functions.csv index 9505cb79c..d76ca2044 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/_helpers/duckdb_catalog/functions.csv +++ b/crates/sqlquack-semantics/src/duckdb/profile/_helpers/duckdb_catalog/functions.csv @@ -829,6 +829,8 @@ julian,scalar,1,false kahan_sum,aggregate,1,false kurtosis,aggregate,1,false kurtosis_pop,aggregate,1,false +lag,window,1,false +lag,window,2,false lag,window,3,false lance_fts,table,,false lance_hybrid_search,table,,false @@ -838,6 +840,8 @@ last_day,scalar,1,false last_value,window,1,false lcase,scalar,1,false lcm,scalar,2,false +lead,window,1,false +lead,window,2,false lead,window,3,false least,scalar,1,true least_common_multiple,scalar,2,false diff --git a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs index 1fb4fe794..deffc14a7 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs @@ -419,3 +419,51 @@ fn given_duckdb_syntax_its_parser_emits_as_calls_when_checking_then_the_forms_du ); } } + +#[test] +fn given_duckdbs_parser_reads_window_calls_when_checking_arity_then_every_count_duckdb_binds_is_known() + { + // Each verdict is DuckDB 2.0.0's: LAG and LEAD bind one to three arguments, not four. + let test_cases = [ + QueryFindingsTestCase { + description: "LAG and LEAD with one, two and three arguments", + sql: "SELECT LAG(amount) OVER (ORDER BY id) AS a, LAG(amount, 2) OVER (ORDER BY id) AS b, \ + LAG(amount, 1, 0) OVER (ORDER BY id) AS c, LEAD(amount) OVER (ORDER BY id) AS d, \ + LEAD(amount, 1) OVER (ORDER BY id) AS e FROM orders", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a short LEAD inside QUALIFY, COALESCE and CASE", + sql: "SELECT COALESCE(LEAD(amount) OVER (ORDER BY id), 0) AS a, \ + CASE WHEN LAG(status) OVER (ORDER BY id) IS NULL THEN 1 END AS b \ + FROM orders QUALIFY LAG(amount) OVER (ORDER BY id) > 1", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "the other window functions at the counts DuckDB binds", + sql: "SELECT NTH_VALUE(amount, 2) OVER (ORDER BY id) AS a, NTILE(4) OVER (ORDER BY id) AS b, \ + FIRST_VALUE(amount) OVER (ORDER BY id) AS c, LAST_VALUE(amount) OVER (ORDER BY id) AS d, \ + PERCENT_RANK() OVER (ORDER BY id) AS e, CUME_DIST() OVER (ORDER BY id) AS f, \ + ROW_NUMBER() OVER () AS g, RANK() OVER (ORDER BY id) AS h, \ + DENSE_RANK() OVER (ORDER BY id) AS i FROM orders", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "LAG with four arguments is still refused", + sql: "SELECT LAG(amount, 1, 0, 2) OVER (ORDER BY id) AS a FROM orders", + expected_findings: &[( + "E203", + "Invalid arity for function 'LAG': got 4, expected 1, 2, 3", + 11, + )], + }, + ]; + for test_case in test_cases { + assert_eq!( + duckdb_parser_findings(test_case.sql), + owned_findings(test_case.expected_findings), + "{}", + test_case.description + ); + } +} diff --git a/scripts/duckdb_catalogue/_helpers/engine.py b/scripts/duckdb_catalogue/_helpers/engine.py index 7ab494fb7..8b470ffe7 100644 --- a/scripts/duckdb_catalogue/_helpers/engine.py +++ b/scripts/duckdb_catalogue/_helpers/engine.py @@ -7,11 +7,12 @@ from scripts.duckdb_catalogue.constants import ( PARSER_KEYWORDS_QUERY, PROBE_ARGUMENTS, - PROBE_TABLE, SYNTAX_FORM_PROBES, SYNTAX_MARKER, TRANSFORMER_REWRITTEN_NAMES, UNKNOWN_FUNCTION_MARKERS, + WINDOW_KIND, + WINDOW_PROBE, ) @@ -32,7 +33,6 @@ def load_extensions(connection: duckdb.DuckDBPyConnection) -> tuple[list[str], l def special_forms(*, connection: duckdb.DuckDBPyConnection, known: set[str]) -> list[str]: """Keywords and transformed names whose call DuckDB does not refuse as an unknown function.""" - connection.execute(PROBE_TABLE) keywords: list[str] = [row[0] for row in connection.execute(PARSER_KEYWORDS_QUERY).fetchall()] special: list[str] = [] for name in sorted({*keywords, *TRANSFORMER_REWRITTEN_NAMES} - known): @@ -69,3 +69,30 @@ def reaches_past_lookup( or SYNTAX_MARKER in message ) return True + + +def catalogue_rows( + *, connection: duckdb.DuckDBPyConnection, listed: list[tuple[str, str, int | None, bool]] +) -> list[tuple[str, str, int | None, bool]]: + """The listed functions, a fixed-arity window function once per argument count DuckDB binds.""" + rows: list[tuple[str, str, int | None, bool]] = [] + for name, kind, arguments, variadic in listed: + if kind != WINDOW_KIND or arguments is None or variadic: + rows.append((name, kind, arguments, variadic)) + continue + counts: list[int] = window_arities(connection=connection, name=name, listed=arguments) + rows.extend((name, kind, count, variadic) for count in counts or [arguments]) + return rows + + +def window_arities(*, connection: duckdb.DuckDBPyConnection, name: str, listed: int) -> list[int]: + """The argument counts up to `listed` that DuckDB binds for the window function `name`.""" + accepted: list[int] = [] + for count in range(listed + 1): + arguments: str = ", ".join(["a", *["1"] * (count - 1)]) if count else "" + try: + connection.execute(WINDOW_PROBE.format(name=name, arguments=arguments)) + except duckdb.Error: + continue + accepted.append(count) + return accepted diff --git a/scripts/duckdb_catalogue/constants.py b/scripts/duckdb_catalogue/constants.py index 12d05b909..177d49d05 100644 --- a/scripts/duckdb_catalogue/constants.py +++ b/scripts/duckdb_catalogue/constants.py @@ -23,6 +23,8 @@ "rollup": "SELECT a FROM probe GROUP BY ROLLUP (a)", } PROBE_TABLE: str = "CREATE TABLE probe (a INTEGER, b VARCHAR)" +WINDOW_KIND: str = "window" +WINDOW_PROBE: str = "SELECT {name}({arguments}) OVER (ORDER BY a) FROM probe" PROBE_ARGUMENTS: tuple[str, ...] = ("a", "a, 1", "b, b, 1") UNKNOWN_FUNCTION_MARKERS: tuple[str, ...] = ("does not exist", "not in the catalog") SYNTAX_MARKER: str = "Parser Error: syntax error" diff --git a/scripts/duckdb_catalogue/main/extract.py b/scripts/duckdb_catalogue/main/extract.py index d82870f69..b39629a2d 100644 --- a/scripts/duckdb_catalogue/main/extract.py +++ b/scripts/duckdb_catalogue/main/extract.py @@ -10,8 +10,13 @@ import duckdb -from scripts.duckdb_catalogue._helpers.engine import load_extensions, special_forms, syntax_forms -from scripts.duckdb_catalogue.constants import BUILT_IN_FUNCTIONS_QUERY, DATA_DIR +from scripts.duckdb_catalogue._helpers.engine import ( + catalogue_rows, + load_extensions, + special_forms, + syntax_forms, +) +from scripts.duckdb_catalogue.constants import BUILT_IN_FUNCTIONS_QUERY, DATA_DIR, PROBE_TABLE def run_catalog_extract(argv: list[str] | None = None) -> int: @@ -23,9 +28,10 @@ def run_catalog_extract(argv: list[str] | None = None) -> int: config={"extension_directory": str(options.extension_directory)} ) loaded, unloadable = load_extensions(connection) - rows: list[tuple[str, str, int | None, bool]] = connection.execute( - BUILT_IN_FUNCTIONS_QUERY - ).fetchall() + connection.execute(PROBE_TABLE) + rows: list[tuple[str, str, int | None, bool]] = catalogue_rows( + connection=connection, listed=connection.execute(BUILT_IN_FUNCTIONS_QUERY).fetchall() + ) known: set[str] = {row[0] for row in rows} special: list[str] = special_forms(connection=connection, known=known) syntax: list[str] = syntax_forms(connection) From e0640d10270162b7db8115801a4e1867cf629a23 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:16:30 +0000 Subject: [PATCH 2/4] fix(semantics): let a derived table inside a LATERAL subquery read enclosing items Name resolution gave a derived table that is not LATERAL no enclosing scope, so a derived table nested in a LATERAL subquery could not read the FROM items before that subquery and reported them as unknown aliases. It now reads the enclosing queries, as lineage tracing already does, while only a LATERAL item also reads its left siblings. --- .../profile/tests/test_statement_findings.rs | 43 +++++++++++++++++++ .../src/lineage/_helpers/findings/levels.rs | 5 ++- .../profile/tests/test_statement_findings.rs | 39 +++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs index deffc14a7..5f602040d 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs @@ -420,6 +420,49 @@ fn given_duckdb_syntax_its_parser_emits_as_calls_when_checking_then_the_forms_du } } +#[test] +fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_reads_the_enclosing_items() + { + // Each verdict is DuckDB 2.0.0's: the derived tables reading p run, the one reading q does not. + let test_cases = [ + QueryFindingsTestCase { + description: "a comma-joined LATERAL whose nested derived table reads the item before it", + sql: "SELECT k.n FROM orders AS p, \ + LATERAL (SELECT f.u AS n FROM (SELECT UNNEST(p.items) AS u) AS f) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "the same body under CROSS JOIN LATERAL", + sql: "SELECT k.n FROM orders AS p \ + CROSS JOIN LATERAL (SELECT f.u AS n FROM (SELECT UNNEST(p.items) AS u) AS f) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a scalar subquery whose derived table reads the outer query", + sql: "SELECT (SELECT f.u FROM (SELECT p.items AS u) AS f) AS n FROM orders AS p", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a nested derived table still reports an alias no enclosing query has", + sql: "SELECT k.n FROM orders AS p, \ + LATERAL (SELECT f.u AS n FROM (SELECT q.items AS u) AS f) AS k", + expected_findings: &[( + "E222", + "Unknown table or alias 'q' referenced by column 'items'", + 67, + )], + }, + ]; + for test_case in test_cases { + assert_eq!( + duckdb_parser_findings(test_case.sql), + owned_findings(test_case.expected_findings), + "{}", + test_case.description + ); + } +} + #[test] fn given_duckdbs_parser_reads_window_calls_when_checking_arity_then_every_count_duckdb_binds_is_known() { diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs b/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs index 94b90cc87..4f3481b82 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs @@ -224,7 +224,10 @@ fn select_findings( let rules: &dyn DialectRules = dialect_rules(context.profile); let anywhere: bool = rules.outputs_read_anywhere(); for item in from_items(select) { - let reads: Option<&LineageScope<'_>> = lateral(item).then_some(&scope); + let reads: Option<&LineageScope<'_>> = match lateral(item) { + true => Some(&scope), + false => outer, + }; let body: Vec = query_findings(unannotated(item), (&ctes, reads, aliases), context); if let Some(derived) = derived_key(unannotated(item), context) diff --git a/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs index f10de51ce..e5d95f79e 100644 --- a/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs @@ -74,3 +74,42 @@ fn given_a_postgres_relation_with_a_finding_when_resolving_names_then_only_its_f ); } } + +#[test] +fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_reads_the_enclosing_items() + { + // Each verdict is PostgreSQL 18.6's: a nested derived table may read the item before the + // LATERAL subquery, but a derived table that is not LATERAL may not read its siblings. + let test_cases = [ + QueryFindingsTestCase { + description: "a comma-joined LATERAL whose nested derived table reads the item before it", + sql: "SELECT k.n FROM orders AS p, \ + LATERAL (SELECT f.u AS n FROM (SELECT p.items AS u) AS f) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "the same body under CROSS JOIN LATERAL", + sql: "SELECT k.n FROM orders AS p \ + CROSS JOIN LATERAL (SELECT f.u AS n FROM (SELECT p.items AS u) AS f) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a derived table that is not LATERAL still cannot read its sibling", + sql: "SELECT k.n FROM orders AS p, (SELECT p.items AS n) AS k", + expected_findings: &[( + "E222", + "Unknown table or alias 'p' referenced by column 'items'", + 37, + )], + }, + ]; + let schema = duckdb_shop_schema(); + for test_case in test_cases { + assert_eq!( + found_findings(test_case.sql, (Dialect::PostgreSQL, &POSTGRES, &schema)), + owned_findings(test_case.expected_findings), + "{}", + test_case.description + ); + } +} From 69ba2847a5516b4b4fabc7c4be1cde284ae0fef6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:35:09 +0000 Subject: [PATCH 3/4] fix(semantics): let a FROM item read only the items before it A LATERAL item was checked against every FROM item of its level, so its body, and any derived table nested in it, could read items to its right and itself without an error, though engines refuse both. It now reads only the items before it, then the enclosing queries. DuckDB also runs a derived table without LATERAL as lateral when it reads an item before it. Name resolution and lineage now let it do so under DuckDB only; PostgreSQL and the other dialects still refuse it. --- .../src/bigquery/profile/models.rs | 4 ++ .../src/clickhouse/profile/models.rs | 4 ++ .../src/databricks/profile/models.rs | 4 ++ .../src/duckdb/profile/models.rs | 4 ++ .../profile/tests/test_query_lineage.rs | 8 +++ .../profile/tests/test_statement_findings.rs | 70 +++++++++++++++++-- .../src/lineage/_helpers/findings/levels.rs | 42 +++++++++-- .../src/lineage/_helpers/tracing/queries.rs | 3 + .../src/lineage/_helpers/tracing/sources.rs | 26 +++++-- .../sqlquack-semantics/src/lineage/models.rs | 8 +++ .../sqlquack-semantics/src/lineage/types.rs | 3 + .../src/postgres/profile/models.rs | 4 ++ .../profile/tests/test_statement_findings.rs | 62 ++++++++++++++-- .../src/snowflake/profile/models.rs | 4 ++ .../src/sqlite/profile/models.rs | 4 ++ .../src/sqlserver/profile/models.rs | 4 ++ 16 files changed, 233 insertions(+), 21 deletions(-) diff --git a/crates/sqlquack-semantics/src/bigquery/profile/models.rs b/crates/sqlquack-semantics/src/bigquery/profile/models.rs index 37ac829f6..e012b65ab 100644 --- a/crates/sqlquack-semantics/src/bigquery/profile/models.rs +++ b/crates/sqlquack-semantics/src/bigquery/profile/models.rs @@ -110,6 +110,10 @@ impl DialectRules for BigQueryRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/clickhouse/profile/models.rs b/crates/sqlquack-semantics/src/clickhouse/profile/models.rs index d9a0ad571..c5c5bed61 100644 --- a/crates/sqlquack-semantics/src/clickhouse/profile/models.rs +++ b/crates/sqlquack-semantics/src/clickhouse/profile/models.rs @@ -125,6 +125,10 @@ impl DialectRules for ClickHouseRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/databricks/profile/models.rs b/crates/sqlquack-semantics/src/databricks/profile/models.rs index 9898b855e..cdc35fc72 100644 --- a/crates/sqlquack-semantics/src/databricks/profile/models.rs +++ b/crates/sqlquack-semantics/src/databricks/profile/models.rs @@ -115,6 +115,10 @@ impl DialectRules for DatabricksRules { true } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/duckdb/profile/models.rs b/crates/sqlquack-semantics/src/duckdb/profile/models.rs index 9ac5bbe32..f1cc261d8 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/models.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/models.rs @@ -116,6 +116,10 @@ impl DialectRules for DuckDbRules { false } + fn derived_tables_read_left_items(&self) -> bool { + true + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_query_lineage.rs b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_query_lineage.rs index b1f954282..00786ae49 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_query_lineage.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_query_lineage.rs @@ -23,6 +23,14 @@ use crate::lineage::tests::test_types::QueryLineageTestCase; #[test] fn given_a_query_when_tracing_lineage_then_each_output_names_its_sources_and_transform() { let test_cases = [ + QueryLineageTestCase { + description: "a derived table without LATERAL reads the item before it, as DuckDB runs it", + sql: "SELECT k.n FROM orders AS p, (SELECT p.status AS n) AS k", + dialect: Dialect::DuckDB, + profile: &DUCKDB, + expected_outputs: &[("n", TransformKind::Direct, &["orders.status"])], + expected_open_star: false, + }, QueryLineageTestCase { description: "DuckDB names a pivoted column by its value's text, without quotes", sql: "SELECT * FROM (SELECT customer_id, status, amount FROM orders) PIVOT (SUM(amount) FOR status IN ('open', 'shipped'))", diff --git a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs index 5f602040d..8d2e7c60c 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs @@ -421,9 +421,9 @@ fn given_duckdb_syntax_its_parser_emits_as_calls_when_checking_then_the_forms_du } #[test] -fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_reads_the_enclosing_items() +fn given_from_items_that_read_each_other_when_resolving_names_then_each_reads_only_the_items_before_it() { - // Each verdict is DuckDB 2.0.0's: the derived tables reading p run, the one reading q does not. + // Each verdict is DuckDB 2.0.0's, which also runs a derived table without LATERAL as lateral. let test_cases = [ QueryFindingsTestCase { description: "a comma-joined LATERAL whose nested derived table reads the item before it", @@ -443,15 +443,75 @@ fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_ expected_findings: &[], }, QueryFindingsTestCase { - description: "a nested derived table still reports an alias no enclosing query has", + description: "a LATERAL body reading the item after it", + sql: "SELECT k.n FROM orders AS p, LATERAL (SELECT q.id AS n) AS k, orders AS q", + expected_findings: &[( + "E222", + "Unknown table or alias 'q' referenced by column 'id'", + 45, + )], + }, + QueryFindingsTestCase { + description: "a derived table nested in a LATERAL reading the item after it", sql: "SELECT k.n FROM orders AS p, \ - LATERAL (SELECT f.u AS n FROM (SELECT q.items AS u) AS f) AS k", + LATERAL (SELECT f.u AS n FROM (SELECT q.id AS u) AS f) AS k, orders AS q", expected_findings: &[( "E222", - "Unknown table or alias 'q' referenced by column 'items'", + "Unknown table or alias 'q' referenced by column 'id'", 67, )], }, + QueryFindingsTestCase { + description: "a LATERAL body reading itself", + sql: "SELECT k.n FROM orders AS p, LATERAL (SELECT k.n AS n) AS k", + expected_findings: &[( + "E222", + "Unknown table or alias 'k' referenced by column 'n'", + 45, + )], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL reading the item before it", + sql: "SELECT k.n FROM orders AS p, (SELECT p.items AS n) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a derived table nested in one without LATERAL reading the item before it", + sql: "SELECT k.n FROM orders AS p CROSS JOIN (SELECT f.u AS n FROM (SELECT p.id AS u) AS f) AS k", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL reading the item after it", + sql: "SELECT k.n FROM orders AS p, (SELECT q.id AS n) AS k, orders AS q", + expected_findings: &[( + "E222", + "Unknown table or alias 'q' referenced by column 'id'", + 37, + )], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL after LEFT JOIN reading the item before it", + sql: "SELECT k.n FROM orders AS p LEFT JOIN (SELECT p.id AS n) AS k ON TRUE", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL after RIGHT JOIN, which DuckDB never runs as lateral", + sql: "SELECT k.n FROM orders AS p RIGHT JOIN (SELECT p.id AS n) AS k ON TRUE", + expected_findings: &[( + "E222", + "Unknown table or alias 'p' referenced by column 'id'", + 47, + )], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL after FULL JOIN, which DuckDB never runs as lateral", + sql: "SELECT k.n FROM orders AS p FULL JOIN (SELECT p.id AS n) AS k ON TRUE", + expected_findings: &[( + "E222", + "Unknown table or alias 'p' referenced by column 'id'", + 46, + )], + }, ]; for test_case in test_cases { assert_eq!( diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs b/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs index 4f3481b82..d9847850d 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/findings/levels.rs @@ -17,11 +17,11 @@ use crate::lineage::_helpers::findings::checks::{ }; use crate::lineage::_helpers::tracing::expressions::date_part_argument; use crate::lineage::_helpers::tracing::queries::{ - level_scope, output_key, unannotated, with_ctes_visiting, + empty_scope, level_scope, output_key, unannotated, with_ctes_visiting, }; use crate::lineage::_helpers::tracing::resolution::key; use crate::lineage::_helpers::tracing::reuse::{Reach, Reuse, reusing}; -use crate::lineage::constants::COLUMNS_FUNCTION; +use crate::lineage::constants::{COLUMNS_FUNCTION, OUTER_RIGHT_JOINS}; use crate::lineage::models::Engine; use crate::lineage::models::NameFinding; use crate::lineage::models::{LineageContext, LineageScope, NamedQuery}; @@ -160,6 +160,23 @@ fn unread_ctes(owner: &Expression, with: &With, context: LineageContext<'_>) -> .collect() } +/// The items before the `index`th FROM item, then `outer`; none when no item comes before it. +fn left_scope<'a>( + scope: &LineageScope<'_>, + index: usize, + (ctes, outer): (&'a [NamedQuery], Option<&'a LineageScope<'a>>), +) -> Option> { + let start: usize = scope + .starts + .get(index) + .map_or(scope.sources.len(), |start| *start); + (start > 0).then(|| LineageScope { + sources: scope.sources[..start].to_vec(), + merged: scope.merged.clone(), + ..empty_scope(ctes, outer) + }) +} + /// The key of a derived table in FROM, by its alias; none when `item` is not a derived table. fn derived_key(item: &Expression, context: LineageContext<'_>) -> Option> { match item { @@ -223,11 +240,22 @@ fn select_findings( let postgres: bool = Engine::of(context) == Engine::Postgres; let rules: &dyn DialectRules = dialect_rules(context.profile); let anywhere: bool = rules.outputs_read_anywhere(); - for item in from_items(select) { - let reads: Option<&LineageScope<'_>> = match lateral(item) { - true => Some(&scope), - false => outer, - }; + let left_reading: bool = rules.derived_tables_read_left_items(); + let listed: usize = select + .from + .as_ref() + .map_or(0, |from| from.expressions.len()); + for (index, item) in from_items(select).into_iter().enumerate() { + let implicit: bool = left_reading + && derived_key(unannotated(item), context).is_some() + && index + .checked_sub(listed) + .and_then(|join| select.joins.get(join)) + .is_none_or(|join| !OUTER_RIGHT_JOINS.contains(&join.kind)); + let left: Option> = (lateral(item) || implicit) + .then(|| left_scope(&scope, index, (&ctes, outer))) + .flatten(); + let reads: Option<&LineageScope<'_>> = left.as_ref().or(outer); let body: Vec = query_findings(unannotated(item), (&ctes, reads, aliases), context); if let Some(derived) = derived_key(unannotated(item), context) diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/tracing/queries.rs b/crates/sqlquack-semantics/src/lineage/_helpers/tracing/queries.rs index 2995cd39c..e0490ba41 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/tracing/queries.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/tracing/queries.rs @@ -129,6 +129,7 @@ pub(crate) fn empty_scope<'a>( non_null: Vec::new(), traced: RefCell::default(), shared: None, + starts: Vec::new(), } } @@ -499,6 +500,7 @@ fn level_scope_and_rows<'a>( sources, merged, rows, + starts, } = match outer { None => reused_items(std::ptr::from_ref(select) as usize, ctes, || { from_sources(select, ctes, None, context) @@ -508,6 +510,7 @@ fn level_scope_and_rows<'a>( let mut scope = LineageScope { sources, merged, + starts, ..empty_scope(ctes, outer) }; scope.windows = named_windows(select, &scope, context); diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/tracing/sources.rs b/crates/sqlquack-semantics/src/lineage/_helpers/tracing/sources.rs index 6bad64f7a..8c2798e67 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/tracing/sources.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/tracing/sources.rs @@ -35,15 +35,18 @@ pub(crate) fn from_sources( outer: Option<&LineageScope<'_>>, context: LineageContext<'_>, ) -> FromItems { - let mut items: Vec<(&Expression, Option<&Join>)> = Vec::new(); + let mut items: Vec<(&Expression, Option<&Join>, bool)> = Vec::new(); for item in select.from.iter().flat_map(|from| from.expressions.iter()) { - items.extend(flattened(item, None)); + items.extend(starting(flattened(item, None))); } for join in &select.joins { - items.extend(flattened(&join.this, Some(join))); + items.extend(starting(flattened(&join.this, Some(join)))); } let mut from = FromItems::default(); - for (item, join) in items { + for (item, join, starts) in items { + if starts { + from.starts.push(from.sources.len()); + } if join.is_some_and(|join| OUTER_RIGHT_JOINS.contains(&join.kind)) { for key in &mut from.merged { key.lineage.nullability = Nullability::Nullable; @@ -93,6 +96,16 @@ pub(crate) fn from_sources( from } +/// One FROM item's flattened parts, the first marked as where the item starts. +fn starting<'a>( + parts: Vec<(&'a Expression, Option<&'a Join>)>, +) -> impl Iterator, bool)> { + parts + .into_iter() + .enumerate() + .map(|(index, (item, join))| (item, join, index == 0)) +} + /// A LATERAL VIEW's columns under its alias, untyped; open when it names no columns. fn lateral_view_source( view: &LateralView, @@ -278,7 +291,10 @@ fn item_sources_level( match item { Expression::Table(table) => vec![table_source(table, scope, context)], Expression::Subquery(subquery) => { - let outer = if subquery.lateral { + let outer = if subquery.lateral + || (!scope.sources.is_empty() + && dialect_rules(context.profile).derived_tables_read_left_items()) + { Some(scope) } else { scope.outer diff --git a/crates/sqlquack-semantics/src/lineage/models.rs b/crates/sqlquack-semantics/src/lineage/models.rs index 827b73ed8..2d3d6ed4c 100644 --- a/crates/sqlquack-semantics/src/lineage/models.rs +++ b/crates/sqlquack-semantics/src/lineage/models.rs @@ -181,6 +181,8 @@ pub(crate) struct LineageScope<'a> { pub(crate) traced: RefCell>, /// Lineages that read no SELECT alias, shared by every scope of this level analysed alike. pub(crate) shared: Option>>>, + /// How many of `sources` the items before each FROM item give, in FROM order. + pub(crate) starts: Vec, } /// A query's outputs while resolving; `open` when it also offers columns not known by name. @@ -216,6 +218,8 @@ pub(crate) struct FromItems { pub(crate) sources: Vec, pub(crate) merged: Vec, pub(crate) rows: RowReads, + /// How many sources the items before each FROM item give, in FROM order. + pub(crate) starts: Vec, } /// The columns that decide a query's rows, and whether any such column could not be traced. @@ -315,6 +319,10 @@ impl DialectRules for SharedRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/lineage/types.rs b/crates/sqlquack-semantics/src/lineage/types.rs index 2c033c650..74722c0f7 100644 --- a/crates/sqlquack-semantics/src/lineage/types.rs +++ b/crates/sqlquack-semantics/src/lineage/types.rs @@ -111,6 +111,9 @@ pub(crate) trait DialectRules: Sync { /// Whether a SELECT may add rows through LATERAL VIEW clauses. fn has_lateral_views(&self) -> bool; + /// Whether a derived table without LATERAL still reads the FROM items before it, as DuckDB's does. + fn derived_tables_read_left_items(&self) -> bool; + /// Whether an ASOF JOIN keeps unmatched left rows, NULL-filling the right. fn as_of_keeps_unmatched_left(&self) -> bool; diff --git a/crates/sqlquack-semantics/src/postgres/profile/models.rs b/crates/sqlquack-semantics/src/postgres/profile/models.rs index d3b7a192f..76d74ac21 100644 --- a/crates/sqlquack-semantics/src/postgres/profile/models.rs +++ b/crates/sqlquack-semantics/src/postgres/profile/models.rs @@ -117,6 +117,10 @@ impl DialectRules for PostgresRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs index e5d95f79e..ebff80d1d 100644 --- a/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/postgres/profile/tests/test_statement_findings.rs @@ -1,6 +1,8 @@ use sqlquack_syntax::Dialect; use crate::dialects::constants::POSTGRES; +use crate::lineage::_helpers::findings::statement::statement_findings; +use crate::lineage::models::LineageContext; use crate::lineage::tests::helpers::{duckdb_shop_schema, found_findings, owned_findings}; use crate::lineage::tests::test_types::QueryFindingsTestCase; @@ -76,10 +78,9 @@ fn given_a_postgres_relation_with_a_finding_when_resolving_names_then_only_its_f } #[test] -fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_reads_the_enclosing_items() +fn given_from_items_that_read_each_other_when_resolving_names_then_each_reads_only_the_items_before_it() { - // Each verdict is PostgreSQL 18.6's: a nested derived table may read the item before the - // LATERAL subquery, but a derived table that is not LATERAL may not read its siblings. + // Each verdict is PostgreSQL 18.6's: only a LATERAL item reads the items before it. let test_cases = [ QueryFindingsTestCase { description: "a comma-joined LATERAL whose nested derived table reads the item before it", @@ -94,7 +95,26 @@ fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_ expected_findings: &[], }, QueryFindingsTestCase { - description: "a derived table that is not LATERAL still cannot read its sibling", + description: "a LATERAL body reading the item after it", + sql: "SELECT k.n FROM orders AS p, LATERAL (SELECT q.id AS n) AS k, orders AS q", + expected_findings: &[( + "E222", + "Unknown table or alias 'q' referenced by column 'id'", + 45, + )], + }, + QueryFindingsTestCase { + description: "a derived table nested in a LATERAL reading the item after it", + sql: "SELECT k.n FROM orders AS p, \ + LATERAL (SELECT f.u AS n FROM (SELECT q.id AS u) AS f) AS k, orders AS q", + expected_findings: &[( + "E222", + "Unknown table or alias 'q' referenced by column 'id'", + 67, + )], + }, + QueryFindingsTestCase { + description: "a derived table without LATERAL reading the item before it", sql: "SELECT k.n FROM orders AS p, (SELECT p.items AS n) AS k", expected_findings: &[( "E222", @@ -102,14 +122,48 @@ fn given_a_derived_table_inside_a_lateral_subquery_when_resolving_names_then_it_ 37, )], }, + QueryFindingsTestCase { + description: "a derived table nested in one without LATERAL reading the item before it", + sql: "SELECT k.n FROM orders AS p CROSS JOIN (SELECT f.u AS n FROM (SELECT p.id AS u) AS f) AS k", + expected_findings: &[( + "E222", + "Unknown table or alias 'p' referenced by column 'id'", + 69, + )], + }, ]; let schema = duckdb_shop_schema(); for test_case in test_cases { + let statement = sqlquack_syntax_postgres::parse_strict(test_case.sql) + .unwrap_or_else(|failure| panic!("{}: {failure:?}", test_case.description)) + .statements + .remove(0); + let context = LineageContext { + profile: &POSTGRES, + schema: &schema, + sql: Some(test_case.sql), + }; + let native: Vec<(&'static str, String, usize)> = statement_findings(&statement, context) + .into_iter() + .map(|finding| { + ( + finding.code, + finding.message, + finding.span.map_or(0, |span| span.start), + ) + }) + .collect(); assert_eq!( found_findings(test_case.sql, (Dialect::PostgreSQL, &POSTGRES, &schema)), owned_findings(test_case.expected_findings), "{}", test_case.description ); + assert_eq!( + native, + owned_findings(test_case.expected_findings), + "{}, PostgreSQL's parser", + test_case.description + ); } } diff --git a/crates/sqlquack-semantics/src/snowflake/profile/models.rs b/crates/sqlquack-semantics/src/snowflake/profile/models.rs index ed0253414..544d6e12a 100644 --- a/crates/sqlquack-semantics/src/snowflake/profile/models.rs +++ b/crates/sqlquack-semantics/src/snowflake/profile/models.rs @@ -155,6 +155,10 @@ impl DialectRules for SnowflakeRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { true } diff --git a/crates/sqlquack-semantics/src/sqlite/profile/models.rs b/crates/sqlquack-semantics/src/sqlite/profile/models.rs index c605a391c..047ce4706 100644 --- a/crates/sqlquack-semantics/src/sqlite/profile/models.rs +++ b/crates/sqlquack-semantics/src/sqlite/profile/models.rs @@ -110,6 +110,10 @@ impl DialectRules for SqliteRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } diff --git a/crates/sqlquack-semantics/src/sqlserver/profile/models.rs b/crates/sqlquack-semantics/src/sqlserver/profile/models.rs index 720d6b57c..0e3b0ddbe 100644 --- a/crates/sqlquack-semantics/src/sqlserver/profile/models.rs +++ b/crates/sqlquack-semantics/src/sqlserver/profile/models.rs @@ -117,6 +117,10 @@ impl DialectRules for SqlServerRules { false } + fn derived_tables_read_left_items(&self) -> bool { + false + } + fn as_of_keeps_unmatched_left(&self) -> bool { false } From 6a55dfb5a46cf10dfd1d26a60c4af6877ca2f0f6 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 00:38:29 +0000 Subject: [PATCH 4/4] fix(semantics): stop reading a lambda parameter as an ungrouped column The grouping check walked into lambda bodies and reported a lambda's own parameter, such as e in list_filter(ARRAY_AGG(x), lambda e: e <> 'null'), as a column that must be grouped. A parameter is bound only inside its lambda's body, so it is now exempt there, while outer columns in the body and a column of the same name outside the lambda are still checked. --- .../profile/tests/test_statement_findings.rs | 78 +++++++++++++++++++ .../lineage/_helpers/checks/scope_rules.rs | 67 ++++++++++++++-- 2 files changed, 138 insertions(+), 7 deletions(-) diff --git a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs index 8d2e7c60c..01755aae8 100644 --- a/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs +++ b/crates/sqlquack-semantics/src/duckdb/profile/tests/test_statement_findings.rs @@ -523,6 +523,84 @@ fn given_from_items_that_read_each_other_when_resolving_names_then_each_reads_on } } +#[test] +fn given_a_lambda_in_a_grouped_select_when_checking_grouping_then_only_outer_columns_must_be_grouped() + { + // Each verdict is DuckDB 2.0.0's. + let test_cases = [ + QueryFindingsTestCase { + description: "a lambda parameter, which is no column", + sql: "SELECT o.customer_id, len(list_filter(ARRAY_AGG(o.status), lambda e: e <> \ + 'null')) AS c FROM orders AS o GROUP BY o.customer_id", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a lambda parameter cast over a JSON list of an aggregate", + sql: "SELECT o.customer_id, len(list_filter(CAST(to_json(ARRAY_AGG(o.status)) AS \ + JSON[]), lambda e: CAST(e AS VARCHAR) <> 'null')) AS c FROM orders AS o GROUP BY \ + o.customer_id", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a grouped outer column inside the lambda body", + sql: "SELECT o.customer_id, len(list_filter(ARRAY_AGG(o.status), lambda e: e <> \ + CAST(o.customer_id AS VARCHAR))) AS c FROM orders AS o GROUP BY o.customer_id", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "an ungrouped outer column inside the lambda body", + sql: "SELECT o.customer_id, len(list_filter(ARRAY_AGG(o.status), lambda e: e <> \ + o.status)) AS c FROM orders AS o GROUP BY o.customer_id", + expected_findings: &[( + "E230", + "Column must be grouped or used in an aggregate function", + 76, + )], + }, + QueryFindingsTestCase { + description: "a parameter's name read outside its lambda is a column", + sql: "SELECT o.customer_id, concat(list_filter(ARRAY_AGG(o.status), lambda status: \ + status <> 'x')[1], status) AS c FROM orders AS o GROUP BY o.customer_id", + expected_findings: &[( + "E230", + "Column must be grouped or used in an aggregate function", + 96, + )], + }, + QueryFindingsTestCase { + description: "a lambda field the FROM item of that name also has is that item's column", + sql: "SELECT list_transform([{'amount': 99}], lambda o: o.amount) AS v FROM orders AS o GROUP BY o.status", + expected_findings: &[( + "E230", + "Column must be grouped or used in an aggregate function", + 52, + )], + }, + QueryFindingsTestCase { + description: "a lambda field only the parameter has", + sql: "SELECT list_transform([{'m': 99}], lambda o: o.m) AS v FROM orders AS o GROUP BY o.status", + expected_findings: &[], + }, + QueryFindingsTestCase { + description: "a JSON arrow is no lambda, so its column must be grouped", + sql: "SELECT o.items -> o.status AS v FROM orders AS o GROUP BY o.status", + expected_findings: &[( + "E230", + "Column must be grouped or used in an aggregate function", + 9, + )], + }, + ]; + for test_case in test_cases { + assert_eq!( + duckdb_parser_findings(test_case.sql), + owned_findings(test_case.expected_findings), + "{}", + test_case.description + ); + } +} + #[test] fn given_duckdbs_parser_reads_window_calls_when_checking_arity_then_every_count_duckdb_binds_is_known() { diff --git a/crates/sqlquack-semantics/src/lineage/_helpers/checks/scope_rules.rs b/crates/sqlquack-semantics/src/lineage/_helpers/checks/scope_rules.rs index 653a05a11..7fa930215 100644 --- a/crates/sqlquack-semantics/src/lineage/_helpers/checks/scope_rules.rs +++ b/crates/sqlquack-semantics/src/lineage/_helpers/checks/scope_rules.rs @@ -1008,9 +1008,10 @@ fn grouping( } let mut findings: Vec = Vec::new(); for (expression, named) in read { - let mut pending: Vec<&Expression> = vec![unalias(expression)]; + let mut lambdas: Vec<(usize, Vec)> = vec![(0, Vec::new())]; + let mut pending: Vec<(&Expression, usize)> = vec![(unalias(expression), 0)]; let mut expanded: HashSet = HashSet::new(); - while let Some(current) = pending.pop() { + while let Some((current, frame)) = pending.pop() { if query_start(current) || is_aggregate(current) || matches!(current, Expression::WithinGroup(group) if is_aggregate(&group.this)) @@ -1021,8 +1022,21 @@ fn grouping( { continue; } + if let Expression::Lambda(lambda) = current + && (lambda.colon || Engine::of(level.context) != Engine::DuckDB) + { + let names: Vec = lambda + .parameters + .iter() + .map(|name| level.key(name)) + .collect(); + lambdas.push((frame, names)); + pending.push((&lambda.body, lambdas.len() - 1)); + continue; + } if let Expression::Column(column) = current { - if grouped(&column_key(column, level)) + if lambda_parameter(column, (&lambdas, frame), level) + || grouped(&column_key(column, level)) || keyword_function(column, level.scope, level.context) { continue; @@ -1031,7 +1045,7 @@ fn grouping( let name: String = level.key(&column.name); if let Some(alias) = aliases.get(&name) { if expanded.insert(name) { - pending.push(alias); + pending.push((alias, frame)); } continue; } @@ -1040,23 +1054,62 @@ fn grouping( break; } if let Expression::WindowFunction(window) = current { - pending.extend(window.this.children()); + pending.extend( + window + .this + .children() + .into_iter() + .map(|child| (child, frame)), + ); pending.extend( current .children() .into_iter() - .filter(|child| !std::ptr::eq(*child, &window.this)), + .filter(|child| !std::ptr::eq(*child, &window.this)) + .map(|child| (child, frame)), ); } else { let mut children: Vec<&Expression> = current.children(); children.reverse(); - pending.extend(children); + pending.extend(children.into_iter().map(|child| (child, frame))); } } } findings } +/// Whether `column` or its qualifier names an enclosing lambda's parameter no FROM item shadows. +fn lambda_parameter( + column: &Column, + (lambdas, mut frame): (&[(usize, Vec)], usize), + level: RuleLevel<'_, '_>, +) -> bool { + let name: String = level.key(column.table.as_ref().unwrap_or(&column.name)); + let field: String = level.key(&column.name); + if column.table.is_some() + && level.scope.sources.iter().any(|source| { + source + .key + .as_ref() + .is_some_and(|key| level.context.profile.same_identifier(key, &name)) + && offers(source, &field, level) + }) + { + return false; + } + while frame != 0 { + let (parent, names) = &lambdas[frame]; + if names + .iter() + .any(|parameter| level.context.profile.same_identifier(parameter, &name)) + { + return true; + } + frame = *parent; + } + false +} + /// Snowflake's DISTINCT with ORDER BY a column it does not select. fn distinct_order( (node, select): (&Expression, &Select),