Skip to content

Commit 6a98e89

Browse files
max-sixtyclaude
andcommitted
fix: exclude a tuple's own aliases from this.*
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) <noreply@anthropic.com>
1 parent 78adc65 commit 6a98e89

4 files changed

Lines changed: 60 additions & 2 deletions

File tree

prqlc/prqlc/src/semantic/resolver/functions.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,16 +305,21 @@ impl Resolver<'_> {
305305
// so they can be added to scope, before resolving subsequent elements.
306306

307307
let mut fields_new = Vec::with_capacity(fields.len());
308+
self.in_flight_tuple_aliases.push(Vec::new());
308309
for field in fields {
309310
let field = self.fold_within_namespace(field, &param.name)?;
310311

311312
// add aliased columns into scope
312313
if let Some(alias) = field.alias.clone() {
313314
let id = field.id.unwrap();
314-
self.root_mod.module.insert_frame_col(NS_THIS, alias, id);
315+
self.root_mod
316+
.module
317+
.insert_frame_col(NS_THIS, alias.clone(), id);
318+
self.in_flight_tuple_aliases.last_mut().unwrap().push(alias);
315319
}
316320
fields_new.push(field);
317321
}
322+
self.in_flight_tuple_aliases.pop();
318323

319324
// note that this tuple node has to be resolved itself
320325
// (it's elements are already resolved and so their resolving

prqlc/prqlc/src/semantic/resolver/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ pub struct Resolver<'a> {
2222
/// Sometimes ident closures must be resolved and sometimes not. See [test::test_func_call_resolve].
2323
in_func_call_name: bool,
2424

25+
/// Aliases declared by the tuples currently being resolved, innermost last.
26+
///
27+
/// Tuple fields are resolved one at a time and each alias is inserted into
28+
/// the `this` frame so later fields can reference it (`{b = a + 1, c = b * 2}`).
29+
/// A `this.*` in a later field must not pick those up — the wildcard means the
30+
/// columns of the relation entering the transform, not the ones this tuple is
31+
/// in the middle of defining.
32+
///
33+
/// A field that fails to resolve leaves its entry on the stack, since the
34+
/// first error aborts the whole resolve and drops the resolver.
35+
in_flight_tuple_aliases: Vec<Vec<String>>,
36+
2537
pub id: IdGenerator<usize>,
2638
}
2739

@@ -35,6 +47,7 @@ impl Resolver<'_> {
3547
current_module_path: Vec::new(),
3648
default_namespace: None,
3749
in_func_call_name: false,
50+
in_flight_tuple_aliases: Vec::new(),
3851
id: IdGenerator::new(),
3952
}
4053
}

prqlc/prqlc/src/semantic/resolver/names.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,26 @@ impl Resolver<'_> {
229229
let module_fq_self = decls.into_iter().next().unwrap();
230230

231231
// Materialize into a tuple literal, containing idents.
232-
let fields = self.construct_wildcard_include(&module_fq_self);
232+
let mut fields = self.construct_wildcard_include(&module_fq_self);
233233
log::trace!("resolve_ident_wildcard fields: {fields:?}");
234234

235+
// The enclosing tuple inserts each of its aliases into the `this`
236+
// frame as it resolves them, so later fields can refer to earlier
237+
// ones. `this.*` must not pick those up: it means the columns
238+
// entering the transform, not the ones the tuple is in the middle
239+
// of defining. Including them duplicates a shadowed column, and
240+
// `{z = 5, this.*}` under `tuple_uniq take:late` would resolve `z`
241+
// to a field that `tuple_uniq` then discards.
242+
if module_fq_self.path == [NS_THIS] {
243+
fields.retain(|field| match &field.kind {
244+
ExprKind::Ident(ident) => !self
245+
.in_flight_tuple_aliases
246+
.iter()
247+
.any(|tuple| tuple.contains(&ident.name)),
248+
_ => true,
249+
});
250+
}
251+
235252
// `construct_wildcard_include` groups each input's columns into
236253
// a nested, aliased tuple. For a wildcard we want a flat list of
237254
// column references so that transforms consuming the expansion

prqlc/prqlc/tests/integration/sql.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6131,6 +6131,29 @@ fn test_this_wildcard_computed_column() {
61316131
");
61326132
}
61336133

6134+
#[test]
6135+
fn test_this_wildcard_ignores_sibling_alias() {
6136+
// `this.*` covers the columns entering the transform, not the ones the
6137+
// enclosing tuple is still defining — so the sibling `z = 5` doesn't add a
6138+
// second `z` to the expansion.
6139+
assert_snapshot!(compile(
6140+
r###"
6141+
from foo
6142+
select {x, y, z}
6143+
select {z = 5, this.*}
6144+
"###,
6145+
)
6146+
.unwrap(), @"
6147+
SELECT
6148+
5,
6149+
x,
6150+
y,
6151+
z
6152+
FROM
6153+
foo
6154+
");
6155+
}
6156+
61346157
#[test]
61356158
fn test_select_bare_wildcard() {
61366159
// Regression test for #5694: bare `*` in `select` should produce

0 commit comments

Comments
 (0)