diff --git a/.gitignore b/.gitignore index a58776eda4b5..35e72856a4db 100644 --- a/.gitignore +++ b/.gitignore @@ -54,3 +54,6 @@ vendor # Lychee link checker cache .lycheecache + +# Claude Code local settings (per-developer, never committed) +.claude/settings.local.json diff --git a/prqlc/prqlc/src/semantic/mod.rs b/prqlc/prqlc/src/semantic/mod.rs index 3d9f23396b63..47bb4d2d15cc 100644 --- a/prqlc/prqlc/src/semantic/mod.rs +++ b/prqlc/prqlc/src/semantic/mod.rs @@ -211,6 +211,21 @@ pub mod test { ") } + #[test] + fn test_resolve_this_wildcard() { + // `this.*` should include computed columns, matching bare `this` + // (https://github.com/PRQL/prql/issues/6044). + assert_yaml_snapshot!(parse_resolve_and_lower(r###" + from foo + select { a, b, c = a + b } + select this.* + "###).unwrap().relation.columns, @" + - Single: a + - Single: b + - Single: c + ") + } + #[test] fn test_resolve_04() { assert_yaml_snapshot!(parse_resolve_and_lower(r###" diff --git a/prqlc/prqlc/src/semantic/module.rs b/prqlc/prqlc/src/semantic/module.rs index 34f5f70a1270..5a9b69c4a3c9 100644 --- a/prqlc/prqlc/src/semantic/module.rs +++ b/prqlc/prqlc/src/semantic/module.rs @@ -205,18 +205,18 @@ impl Module { log::trace!("lookup: {ident}"); - let mut res = HashSet::new(); - - res.extend(lookup_in(self, ident.clone())); + // A direct hit in this module always wins over redirects. + let mut res = lookup_in(self, ident.clone()); + if !res.is_empty() { + return res; + } + // Otherwise fall back to following redirects into the module's inputs. for redirect in &self.redirects { log::trace!("... following redirect {redirect}"); let r = lookup_in(self, redirect.clone() + ident.clone()); log::trace!("... result of redirect {redirect}: {r:?}"); - if !r.is_empty() { - res.remove(ident); - res.extend(r); - } + res.extend(r); } res } diff --git a/prqlc/prqlc/src/semantic/resolver/functions.rs b/prqlc/prqlc/src/semantic/resolver/functions.rs index 7e86fbfeda51..e7dee639b7a0 100644 --- a/prqlc/prqlc/src/semantic/resolver/functions.rs +++ b/prqlc/prqlc/src/semantic/resolver/functions.rs @@ -305,16 +305,21 @@ impl Resolver<'_> { // so they can be added to scope, before resolving subsequent elements. let mut fields_new = Vec::with_capacity(fields.len()); + self.in_flight_tuple_aliases.push(Vec::new()); for field in fields { let field = self.fold_within_namespace(field, ¶m.name)?; // add aliased columns into scope if let Some(alias) = field.alias.clone() { let id = field.id.unwrap(); - self.root_mod.module.insert_frame_col(NS_THIS, alias, id); + self.root_mod + .module + .insert_frame_col(NS_THIS, alias.clone(), id); + self.in_flight_tuple_aliases.last_mut().unwrap().push(alias); } fields_new.push(field); } + self.in_flight_tuple_aliases.pop(); // note that this tuple node has to be resolved itself // (it's elements are already resolved and so their resolving diff --git a/prqlc/prqlc/src/semantic/resolver/mod.rs b/prqlc/prqlc/src/semantic/resolver/mod.rs index 3453dd7cb13e..ee7ad5599c95 100644 --- a/prqlc/prqlc/src/semantic/resolver/mod.rs +++ b/prqlc/prqlc/src/semantic/resolver/mod.rs @@ -22,6 +22,18 @@ pub struct Resolver<'a> { /// Sometimes ident closures must be resolved and sometimes not. See [test::test_func_call_resolve]. in_func_call_name: bool, + /// Aliases declared by the tuples currently being resolved, innermost last. + /// + /// Tuple fields are resolved one at a time and each alias is inserted into + /// the `this` frame so later fields can reference it (`{b = a + 1, c = b * 2}`). + /// A `this.*` in a later field must not pick those up — the wildcard means the + /// columns of the relation entering the transform, not the ones this tuple is + /// in the middle of defining. + /// + /// A field that fails to resolve leaves its entry on the stack, since the + /// first error aborts the whole resolve and drops the resolver. + in_flight_tuple_aliases: Vec>, + pub id: IdGenerator, } @@ -35,6 +47,7 @@ impl Resolver<'_> { current_module_path: Vec::new(), default_namespace: None, in_func_call_name: false, + in_flight_tuple_aliases: Vec::new(), id: IdGenerator::new(), } } diff --git a/prqlc/prqlc/src/semantic/resolver/names.rs b/prqlc/prqlc/src/semantic/resolver/names.rs index 01e7ee48af45..156c2d4f7e08 100644 --- a/prqlc/prqlc/src/semantic/resolver/names.rs +++ b/prqlc/prqlc/src/semantic/resolver/names.rs @@ -229,9 +229,34 @@ impl Resolver<'_> { let module_fq_self = decls.into_iter().next().unwrap(); // Materialize into a tuple literal, containing idents. - let fields = self.construct_wildcard_include(&module_fq_self); + let mut fields = self.construct_wildcard_include(&module_fq_self); log::trace!("resolve_ident_wildcard fields: {fields:?}"); + // The enclosing tuple inserts each of its aliases into the `this` + // frame as it resolves them, so later fields can refer to earlier + // ones. `this.*` must not pick those up: it means the columns + // entering the transform, not the ones the tuple is in the middle + // of defining. Including them duplicates a shadowed column, and + // `{z = 5, this.*}` under `tuple_uniq take:late` would resolve `z` + // to a field that `tuple_uniq` then discards. + if module_fq_self.path == [NS_THIS] { + fields.retain(|field| match &field.kind { + ExprKind::Ident(ident) => !self + .in_flight_tuple_aliases + .iter() + .any(|tuple| tuple.contains(&ident.name)), + _ => true, + }); + } + + // `construct_wildcard_include` groups each input's columns into + // a nested, aliased tuple. For a wildcard we want a flat list of + // column references so that transforms consuming the expansion + // (e.g. `sort`) receive scalar columns rather than tuples. + // Without flattening, `this.*` over a relation with computed + // columns yields nested tuples that fail to lower (#6044). + let fields = flatten_wildcard_fields(fields); + // This is just a workaround to return an Expr from this function. // We wrap the expr into DeclKind::Expr and save it into the root module. let cols_expr = Expr { @@ -312,6 +337,21 @@ impl Resolver<'_> { } } +/// Recursively splice the per-input nested tuples produced by +/// [`Resolver::construct_wildcard_include`] into a flat list of column +/// references. Leaf columns carry their full path and inferred wildcards carry +/// a `target_id`, so the input association survives the flattening. +fn flatten_wildcard_fields(fields: Vec) -> Vec { + let mut res = Vec::new(); + for field in fields { + match field.kind { + ExprKind::Tuple(inner) => res.extend(flatten_wildcard_fields(inner)), + _ => res.push(field), + } + } + res +} + fn ambiguous_error(idents: HashSet, replace_name: Option<&String>) -> Error { let all_this = idents.iter().all(|d| d.starts_with_part(NS_THIS)); diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index e1ceffcd902e..3c6de08e5e45 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -6090,6 +6090,70 @@ fn test_sort_this_wildcard() { "); } +#[test] +fn test_this_wildcard_computed_column() { + // `this.*` should reflect the current columns of the pipeline, including + // computed ones, matching bare `this` (#6044). + assert_snapshot!(compile( + r###" + from foo + select {a, b, c = a + b} + select this.* + "###, + ) + .unwrap(), @" + SELECT + a, + b, + a + b AS c + FROM + foo + "); + + assert_snapshot!(compile( + r###" + from foo + select {a, b, c = a + b} + sort this.* + "###, + ) + .unwrap(), @" + SELECT + a, + b, + a + b AS c + FROM + foo + ORDER BY + a, + b, + c + "); +} + +#[test] +fn test_this_wildcard_ignores_sibling_alias() { + // `this.*` covers the columns entering the transform, not the ones the + // enclosing tuple is still defining — so the sibling `z = 5` doesn't add a + // second `z` to the expansion. + assert_snapshot!(compile( + r###" + from foo + select {x, y, z} + select {z = 5, this.*} + "###, + ) + .unwrap(), @" + SELECT + 5, + x, + y, + z + FROM + foo + "); +} + #[test] fn test_select_bare_wildcard() { // Regression test for #5694: bare `*` in `select` should produce