From 1335c32518b57c73e59f88b603e4fb5f4b0b1c19 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:00:11 +0000 Subject: [PATCH 1/6] fix: include computed columns in `this.*` wildcard expansion `this.*` resolved `this._self` through an input redirect to the first input's `_self`, enumerating only that input's columns and dropping computed columns. Keep a module's own `_self` when it has one, and flatten the per-input nested tuples so transforms like `sort` receive scalar columns. Closes #6044 Co-Authored-By: Claude --- .claude/settings.local.json | 1 + prqlc/prqlc/src/semantic/mod.rs | 15 ++++++++ prqlc/prqlc/src/semantic/module.rs | 10 +++++- prqlc/prqlc/src/semantic/resolver/names.rs | 23 ++++++++++++ prqlc/prqlc/tests/integration/sql.rs | 41 ++++++++++++++++++++++ 5 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 000000000000..b28ffd45fbea --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1 @@ +{"permissions":{"defaultMode":"bypassPermissions","allow":["Bash","Edit","Read","Write","Glob","Grep","WebSearch","WebFetch","Task","Skill"]},"skipDangerousModePermissionPrompt":true} 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 4f841bd76162..bde65239f5f7 100644 --- a/prqlc/prqlc/src/semantic/module.rs +++ b/prqlc/prqlc/src/semantic/module.rs @@ -182,11 +182,19 @@ impl Module { res.extend(lookup_in(self, ident.clone())); + // `_self` is a per-module marker present at every level. When this + // module already has its own `_self`, a redirect into an input's + // `_self` must not shadow it — otherwise `this.*` would resolve to the + // first input's module and enumerate only its columns, dropping + // computed columns (https://github.com/PRQL/prql/issues/6044). + let keep_direct_self = + ident.path.is_empty() && ident.name == NS_SELF && res.contains(ident); + 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() { + if !r.is_empty() && !keep_direct_self { res.remove(ident); res.extend(r); } diff --git a/prqlc/prqlc/src/semantic/resolver/names.rs b/prqlc/prqlc/src/semantic/resolver/names.rs index a1c3ddfbbf88..63cf2a016f52 100644 --- a/prqlc/prqlc/src/semantic/resolver/names.rs +++ b/prqlc/prqlc/src/semantic/resolver/names.rs @@ -231,6 +231,14 @@ impl Resolver<'_> { let fields = self.construct_wildcard_include(&module_fq_self); log::trace!("resolve_ident_wildcard fields: {fields:?}"); + // `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 { @@ -253,6 +261,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 0f2cc0ebecc1..67161f3e8e28 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -6052,6 +6052,47 @@ 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_select_bare_wildcard() { // Regression test for #5694: bare `*` in `select` should produce From f3900cf62d34ed7a0f7c29a279288006e49d8821 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:00:46 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .claude/settings.local.json | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.claude/settings.local.json b/.claude/settings.local.json index b28ffd45fbea..26c3e0556e2b 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -1 +1,18 @@ -{"permissions":{"defaultMode":"bypassPermissions","allow":["Bash","Edit","Read","Write","Glob","Grep","WebSearch","WebFetch","Task","Skill"]},"skipDangerousModePermissionPrompt":true} +{ + "permissions": { + "defaultMode": "bypassPermissions", + "allow": [ + "Bash", + "Edit", + "Read", + "Write", + "Glob", + "Grep", + "WebSearch", + "WebFetch", + "Task", + "Skill" + ] + }, + "skipDangerousModePermissionPrompt": true +} From b862b61299884cf470ac2ebe9e3569b799be1ca0 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:05:39 +0000 Subject: [PATCH 3/6] chore: drop accidentally committed .claude/settings.local.json This is a Claude Code local-only config (bypassPermissions) that should not be tracked. Add it to .gitignore to prevent recurrence. Co-Authored-By: Claude --- .claude/settings.local.json | 18 ------------------ .gitignore | 3 +++ 2 files changed, 3 insertions(+), 18 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index 26c3e0556e2b..000000000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "permissions": { - "defaultMode": "bypassPermissions", - "allow": [ - "Bash", - "Edit", - "Read", - "Write", - "Glob", - "Grep", - "WebSearch", - "WebFetch", - "Task", - "Skill" - ] - }, - "skipDangerousModePermissionPrompt": true -} 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 From f9fc18ddd0098a369875677f0618a833ae0824ff Mon Sep 17 00:00:00 2001 From: prql-bot Date: Fri, 26 Jun 2026 18:41:20 +0000 Subject: [PATCH 4/6] refactor: return early from Module::lookup when keeping direct `_self` `keep_direct_self` is loop-invariant, so when it's true the redirect loop can never modify `res`. Return early instead of guarding each iteration, per review feedback from @kgutwin. Co-Authored-By: Claude Opus 4.8 --- prqlc/prqlc/src/semantic/module.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/prqlc/prqlc/src/semantic/module.rs b/prqlc/prqlc/src/semantic/module.rs index bde65239f5f7..4f175d7069cc 100644 --- a/prqlc/prqlc/src/semantic/module.rs +++ b/prqlc/prqlc/src/semantic/module.rs @@ -186,15 +186,20 @@ impl Module { // module already has its own `_self`, a redirect into an input's // `_self` must not shadow it — otherwise `this.*` would resolve to the // first input's module and enumerate only its columns, dropping - // computed columns (https://github.com/PRQL/prql/issues/6044). + // computed columns (https://github.com/PRQL/prql/issues/6044). In that + // case the redirects can't contribute anything we'd want, so return the + // direct match without following them. let keep_direct_self = ident.path.is_empty() && ident.name == NS_SELF && res.contains(ident); + if keep_direct_self { + return res; + } 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() && !keep_direct_self { + if !r.is_empty() { res.remove(ident); res.extend(r); } From 720835000e05bb581ebcba7e9b26b11edc044a79 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Fri, 26 Jun 2026 19:09:41 +0000 Subject: [PATCH 5/6] refactor: prefer direct module hits over redirects in lookup() Replace the narrow `keep_direct_self` special-case with a general rule: a direct hit in a module always wins over its redirects, so `lookup()` returns early when the initial `lookup_in()` finds anything. Redirects are only followed as a fallback when there is no direct match. This is the more general fix suggested in review for #6044 and passes the full prqlc test suite. Co-Authored-By: Claude Opus 4.8 --- prqlc/prqlc/src/semantic/module.rs | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/prqlc/prqlc/src/semantic/module.rs b/prqlc/prqlc/src/semantic/module.rs index 4f175d7069cc..058415db0807 100644 --- a/prqlc/prqlc/src/semantic/module.rs +++ b/prqlc/prqlc/src/semantic/module.rs @@ -178,31 +178,18 @@ impl Module { log::trace!("lookup: {ident}"); - let mut res = HashSet::new(); - - res.extend(lookup_in(self, ident.clone())); - - // `_self` is a per-module marker present at every level. When this - // module already has its own `_self`, a redirect into an input's - // `_self` must not shadow it — otherwise `this.*` would resolve to the - // first input's module and enumerate only its columns, dropping - // computed columns (https://github.com/PRQL/prql/issues/6044). In that - // case the redirects can't contribute anything we'd want, so return the - // direct match without following them. - let keep_direct_self = - ident.path.is_empty() && ident.name == NS_SELF && res.contains(ident); - if keep_direct_self { + // 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 } From 6a98e8996effa0b40e5920bc032f4e7cd5ed2db3 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 26 Jul 2026 07:51:47 -0700 Subject: [PATCH 6/6] fix: exclude a tuple's own aliases from `this.*` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tuple fields resolve 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}`). Now that `this.*` enumerates the `this` frame rather than the first input's sub-module, it also picked those up, so `{z = 5, this.*}` expanded `z` twice — once from the input and once as a reference to the sibling literal. `tuple_uniq take:late` then kept the sibling reference and discarded the literal it pointed at, leaving a dangling target that failed to lower (`test_tuple_uniq`). Track the aliases each in-progress tuple declares and skip them when expanding `this.*`; the wildcard means the columns entering the transform, not the ones the tuple is still defining. Co-Authored-By: Claude Opus 5 (1M context) --- .../prqlc/src/semantic/resolver/functions.rs | 7 +++++- prqlc/prqlc/src/semantic/resolver/mod.rs | 13 +++++++++++ prqlc/prqlc/src/semantic/resolver/names.rs | 19 ++++++++++++++- prqlc/prqlc/tests/integration/sql.rs | 23 +++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) 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 d72bf482d3a7..156c2d4f7e08 100644 --- a/prqlc/prqlc/src/semantic/resolver/names.rs +++ b/prqlc/prqlc/src/semantic/resolver/names.rs @@ -229,9 +229,26 @@ 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 diff --git a/prqlc/prqlc/tests/integration/sql.rs b/prqlc/prqlc/tests/integration/sql.rs index ad4a830ef85f..3c6de08e5e45 100644 --- a/prqlc/prqlc/tests/integration/sql.rs +++ b/prqlc/prqlc/tests/integration/sql.rs @@ -6131,6 +6131,29 @@ fn test_this_wildcard_computed_column() { "); } +#[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