diff --git a/CHANGELOG.md b/CHANGELOG.md index e6ab4d4c68fb..62983127b3d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ **Fixes**: +- Stop silently dropping a select item whose name collides with an identifier + used earlier in the same projection — a table qualifier, or a column of the + same name from another relation. `select {tbl.a, tbl = x.b}` lost its second + column, as did a join between two relations that share a column name. + (@prql-bot, #6181) + **Documentation**: **Web**: diff --git a/prqlc/prqlc/src/sql/gen_projection.rs b/prqlc/prqlc/src/sql/gen_projection.rs index eba54e0c3d62..220e88559950 100644 --- a/prqlc/prqlc/src/sql/gen_projection.rs +++ b/prqlc/prqlc/src/sql/gen_projection.rs @@ -121,14 +121,24 @@ pub(super) fn translate_wildcards(ctx: &AnchorContext, cols: Vec) -> (Vec) { - // Dropping all duplicated identifiers - let mut seen = HashSet::new(); + // Dropping all duplicated identifiers. Qualified references and output + // names are tracked separately: a table qualifier such as the `t` of `t.a` + // lives in a different namespace to an output column name, so it must not + // make a later `… AS t` look like a repeat. + let mut seen_idents = HashSet::new(); + let mut seen_names = HashSet::new(); items.retain(|select_item| match select_item { SelectItem::UnnamedExpr(sql_ast::Expr::CompoundIdentifier(idents)) => { - // If any of the identifiers hadn't been seen yet, retain the expr - idents.iter().any(|ident| seen.insert(ident.clone())) + match idents.as_slice() { + // An unqualified `a` carries no qualifier and is itself the + // output name, so it belongs in the same namespace as aliases. + [only] => seen_names.insert(only.clone()), + // Compare the whole path, so that `t.a` and `u.a` both survive + // while a repeat of `t.a` does not. + _ => seen_idents.insert(idents.clone()), + } } - SelectItem::ExprWithAlias { alias, .. } => seen.insert(alias.clone()), + SelectItem::ExprWithAlias { alias, .. } => seen_names.insert(alias.clone()), _ => true, }); } diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index d9265c1bfc52..f80af1882c18 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -1186,6 +1186,50 @@ fn test_sort_in_nested_join_with_extra_derive_and_select() { ); } +#[test] +fn test_alias_matching_table_qualifier_is_kept() { + // The select-item deduplication used to track table qualifiers and output + // aliases in one set, so `tbl` from `tbl.a` made `x.b AS tbl` look like a + // repeat and the column was dropped from the projection. + assert_snapshot!(compile(r#" + from tbl + join x (==id) + select {tbl.a, tbl = x.b} + "#).unwrap(), @r" + SELECT + tbl.a, + x.b AS tbl + FROM + tbl + INNER JOIN x ON tbl.id = x.id + "); +} + +#[test] +fn test_aggregate_matching_group_key() { + // TODO: this is wrong — the aggregate is missing from the projection. + // + // An unqualified group key occupies the same output-name namespace as the + // aggregate's alias, so the dedup pass drops the aggregate. Emitting both + // is not the fix on its own: they would share the name `a`, and `sort a` + // on top of this then compiles to `ORDER BY a`, which Postgres rejects as + // ambiguous and SQLite/DuckDB bind to the aggregate rather than the group + // key. The real fix belongs in the resolver, which disambiguates a + // same-named pair inside `select` (renaming to `_expr_0`) but not in + // `group`/`aggregate`. + assert_snapshot!(compile(r#" + from tbl + group {a} (aggregate {a = sum b}) + "#).unwrap(), @r" + SELECT + a + FROM + tbl + GROUP BY + a + "); +} + #[test] fn test_sort_in_nested_append() { assert_snapshot!(compile(r#" diff --git a/web/book/tests/documentation/snapshots/documentation__book__reference__stdlib__tuple__tuple-functions__4.snap b/web/book/tests/documentation/snapshots/documentation__book__reference__stdlib__tuple__tuple-functions__4.snap index c0cc4296a999..0bd32353c082 100644 --- a/web/book/tests/documentation/snapshots/documentation__book__reference__stdlib__tuple__tuple-functions__4.snap +++ b/web/book/tests/documentation/snapshots/documentation__book__reference__stdlib__tuple__tuple-functions__4.snap @@ -17,6 +17,7 @@ SELECT invoices.processed, shipments.id, shipments.invoice_id, + shipments.date_of, shipments.shipped_on FROM invoices