fix(unparser): list every relation a join input reads when the input lists no columns of its own (refs spiceai/spiceai#14738)

[grokspice]

Which issue does this PR close?

Refs spiceai/spiceai#14738.

Rationale for this change

In the Join arm each input is walked into the shared SELECT and the items its walk set are popped and concatenated. A table scan with no projection sets no items, and an aliased one sets a bare *, so the list the join assembled no longer matched the plan's schema — a consumer reading the result by position got the other input's columns under this input's names, and DuckDB rejected the empty list outright. The issue's two shapes, before this change (spiceai-55 at cf084aa85; the unparser is identical at spiceai's pin 72cdbc610f), run in DuckDB 1.4.4:

plan	emitted SQL before	DuckDB before	plan's output
LeftSemi(t, u), bare scans	SELECT FROM t WHERE EXISTS (SELECT 1 FROM u WHERE (t.a = u.a))	Parser Error: SELECT clause without selection list	(t.a)
Inner(t, Projection[u.a, u.a * 2 AS d] over u), bare t	SELECT u.a, (u.a * 2) AS d FROM t INNER JOIN u ON (t.a = u.a)	2 columns: (1, 2) (2, 4) (3, 6)	3 columns: (1, 1, 2) (2, 2, 4) (3, 3, 6)

The same happened for a bare scan on either side of an inner, left, right or full join, under a filter, in a nested join, and for an aliased bare scan (SELECT *, u.a, … reads both relations through the *; two aliased scans gave SELECT *, *). Three neighbouring shapes of the same class were found while reviewing the fix and are fixed with it: an aggregate whose list no projection takes emitted its aggregates before its group keys, the reverse of the plan's schema (SELECT sum(t.b), t.a FROM t GROUP BY t.a for schema [t.a, sum(t.b)]); a join input projected to no columns carried its SELECT 1 placeholder into the shared list beside the other input's columns; and a window over a bare scan listed * and its window expression, the * reading the other input too.

What changes are included in this PR?

join_input_select_items replaces the two pop_projections() calls in the Join arm. An input whose walk set nothing, or only an unqualified * (a SelectItem::Wildcard, or the expression item an Expr::Wildcard listed by a projection renders as), contributes one q.* per relation its walk wrote into the FROM, meaning the relation builder and the joins it appended, under the names the FROM gives them. When it is the only relation the FROM holds (the probe side of an EXISTS-style join), it contributes a bare * instead, which is how a scan unparsed on its own renders. An unqualified * inside a longer list (a window input's) is narrowed in place to the same relations, and the explicit items around it are kept. The relations are read from the FROM and not from the plan's schema, so anything the walk derived or renamed is read under the name it has there. That covers a window's derived input (derived_window_input.*) and an alias over a join, which renames only the join's first relation (FROM t AS j INNER JOIN u reads as j.*, u.*). An input that already lists its own items is returned as it was set. One with no columns contributes nothing beside another relation (two such inputs keep the placeholder). One whose walk wrote no relation (a bare union) is refused, and so is one whose output still carries a mark join's mark, which * would drop (spiceai/spiceai#15220). The q.* form is kept even when every input reads whole relations, because a bare * would also read a column-less input's hidden columns and, under USING, would merge the key columns the plan keeps apart. The exception is two relations that answer to the same qualifier, such as main.l AS t beside other.t (both t when the dialect qualifies by table name alone). There t.*, t.* would read both relations twice. If the list is exactly one q.* per relation in FROM order and no join merges columns, the join emits the bare * the base did. Any other list with a colliding q.* is refused, and re-aliasing the relation instead is spiceai/spiceai#15238. The Aggregate arm lists group keys before aggregates when no projection takes the list, which is the order the plan's schema has.

Snapshots that encoded the old lists are re-pinned: seven sub-cases of test_join_with_table_scan_filters (four SELECT * over an aliased scan and a bare scan become "left".*, right_table.*; three listed only the left scan's two columns where the plan has four and gain right_table.*), and twelve projection-less aggregate snapshots whose only change is the group key moving in front of the aggregate.

Are these changes tested?

Twenty-three new tests in plan_to_sql.rs cover the issue's shapes and the shapes found while reviewing the fix. They include both scans bare, a scan under a filter, two aliased scans, a nested join of bare scans beside a projected input, a bare right input of a RIGHT JOIN, and a projection-less aggregate alone and as an aliased join input. They also cover a column-less join input alone and paired with another, a nested join holding a column-less input, two bare scans joined with USING, and windows over a bare scan, a projection, a limit, a filter, a sort, a DISTINCT and a semi or anti join beside a bare scan. The last five cover two relations answering to one qualifier (read through a bare *, or refused under USING, beside a column-less input and beside a listed column), a projection listing an unqualified Expr::Wildcard, an alias over a join as a join input, and a mark join read through *. With the tests committed and datafusion/sql/src reset to origin/spiceai-55, they and the re-pinned snapshots fail (802 passed; 46 failed = the suite's 22 pre-existing failures plus the first 24 test functions, before the window test was added). The later tests were red on the heads before their fixes: t.*, *, … on 17aba5043, t.*, u.*, … over a derived window input on 7e8cde350, and t.*, t.*, t.*, * and j.*, w.* on 0800a1231. On the head cd45aa3c7 (3112eb390 merged with spiceai-55 at ddd5c8ce6), the suite fails only the pre-existing 22: 851 passed; 22 failed, against 828 passed; 22 failed on ddd5c8ce6, with the same 22 test names. Before the merge, 3112eb390 gave 838 passed; 22 failed against 815 passed; 22 failed on e8d8abae9. On cd45aa3c7, cargo +1.97.0 test -p datafusion-sql --lib gives 103 passed, and cargo +1.97.0 clippy -p datafusion-sql --all-targets -- -D warnings and cargo fmt --all -- --check are clean.

Differential check against DuckDB 1.4.4 (Python duckdb): a probe unparses 22 join shapes with plan_to_sql, and an oracle script runs the emitted SQL and a hand-written statement for the same plan over tables matching the plan's schema, comparing column count and rows. Before, on cf084aa85 (16 shapes; J17–J22 were added through the reviews):

FAIL J1:  Parser Error: SELECT clause without selection list      SELECT FROM t WHERE EXISTS (…)
FAIL J2:  columns=2 rows=[(1, 2), (2, 4), (3, 6)]   expected columns=3 rows=[(1, 1, 2), (2, 2, 4), (3, 3, 6)]
FAIL J3:  Parser Error …                                            SELECT FROM t INNER JOIN u ON (t.a = u.a)
FAIL J4:  columns=2   expected columns=3                            SELECT t.a, (t.b + 1) AS c FROM t INNER JOIN u …
FAIL J5:  columns=1   expected columns=3                            SELECT u.a FROM t LEFT OUTER JOIN u …
FAIL J6:  Parser Error …                                            SELECT FROM t WHERE NOT EXISTS (…)
FAIL J10: columns=5   expected columns=4                            SELECT *, u.a, (u.a * 2) AS d FROM t AS x INNER JOIN u …
FAIL J11: columns=2   expected columns=4                            … FROM t INNER JOIN u ON (t.a = u.a) WHERE (t.b > 1)
FAIL J12: columns=2   expected columns=4                            SELECT w.a, (w.a * 2) AS d FROM t INNER JOIN u … INNER JOIN w …
FAIL J13: Parser Error …                                            SELECT FROM t INNER JOIN u … WHERE (t.b > 1)
FAIL J14: columns=6   expected columns=3                            SELECT *, * FROM t AS x INNER JOIN u AS y …
FAIL J15: columns=2   expected columns=3                            SELECT t.a, (t.b + 1) AS c FROM t RIGHT OUTER JOIN u …
FAIL J16: Parser Error …                                            SELECT FROM t FULL JOIN u ON (t.a = u.a)
RESULT: FAIL (3/16 shapes agree with DuckDB 1.4.4)

After, on this branch at 34c21dda7:

J1  SELECT * FROM t WHERE EXISTS (SELECT 1 FROM u WHERE (t.a = u.a))
J2  SELECT t.*, u.a, (u.a * 2) AS d FROM t INNER JOIN u ON (t.a = u.a)
J3  SELECT t.*, u.* FROM t INNER JOIN u ON (t.a = u.a)
J10 SELECT x.*, u.a, (u.a * 2) AS d FROM t AS x INNER JOIN u ON (x.a = u.a)
J12 SELECT t.*, u.*, w.a, (w.a * 2) AS d FROM t INNER JOIN u ON (t.a = u.a) INNER JOIN w ON (t.a = w.a)
J15 SELECT t.a, (t.b + 1) AS c, u.* FROM t RIGHT OUTER JOIN u ON (t.a = u.a)
J17 SELECT u.*, w.* FROM u INNER JOIN t ON true INNER JOIN w ON (u.a = w.a)
J18 SELECT t.*, u.* FROM t INNER JOIN u USING(a)
J19 SELECT t.*, u.*, row_number() OVER (ORDER BY u.a ASC NULLS FIRST ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS rn FROM t INNER JOIN u ON true
J20 SELECT j.*, w.* FROM (SELECT t.a, u.a AS "a:1", u.b FROM t INNER JOIN u ON true) AS j INNER JOIN w ON true
J21 SELECT t.*, derived_window_input.*, row_number() OVER (…) AS rn FROM t INNER JOIN (SELECT u.a FROM u) AS derived_window_input ON true
RESULT: PASS (22/22 shapes agree with DuckDB 1.4.4)

The review threads on 0800a1231 found three regressions against the base. A third probe (17 shapes, default and SqliteDialect) runs each statement in SQLite 3.50.4 (schemas as attached databases) and DuckDB 1.4.4 against a hand-written statement for the same plan. Disagreeing engine runs: base e8d8abae9 24, 0800a1231 36 (on its first 15 shapes), 3112eb390 6. The 6 left on this branch are the two ON t.a = other.t.a shapes. Both engines reject those as ambiguous on the base too, with the identical statement (spiceai/spiceai#15238):

shape	base e8d8abae9	0800a1231	3112eb390
main.l AS t ⋈ other.t (also with u between, or nested on the right)	SELECT * ✓	t.*, t.*: SQLite 4 cols for 2, DuckDB ambiguous	SELECT * ✓
s1.t ⋈ s2.t	SELECT FROM: parse error	4 cols / ambiguous	SELECT * ✓
colliding, with USING(k) / a column-less input / a listed column	3 cols for 4 / 3 for 1 / 4 for 2	8 / 2 / 4 cols, DuckDB ambiguous	refused
t ⋈ projection of Expr::Wildcard over u (either side, or with an extra item)	SELECT * ✓	t.*, *: 3 cols for 2	t.*, u.* ✓
SubqueryAlias(j, t ⋈ u) ⋈ w	SELECT * ✓	j.*, w.*: 2 cols for 3	j.*, u.*, w.* ✓
mark join ⋈ w	SELECT FROM: parse error	t.*, u.*, w.*: u not found	refused
aliased union ⋈ t	SELECT FROM: parse error	t.*, x.* ✓	t.*, x.* ✓

A second, independently written probe of 68 join shapes (nested and interleaved inputs, three-part table names on the default and PostgreSQL dialects, RIGHT/semi/anti/mark joins, limits and filters over bare inputs, derived-table and union inputs, duplicate column names, self-joins) agreed with DuckDB on 56 shapes on 17aba5043, against 7 on an older base. It has not been re-run on this head. The 12 that did not match were not changed by the diff at that point beyond the empty list becoming a wildcard. 3 shapes are an Aggregate directly as a join input, which folds into the join's own SELECT and is now a binder error where the base returned wrong rows. 5 are a mark join with no projection above, which drops its mark column; as a join input that is now refused. 3 are unions and VALUES as bare inputs, which are refused where the base dropped the join or failed internally. The rest are three-part names under CustomDialectBuilder::with_full_qualified_col(true), which render cat.sch.t.*; DuckDB's parser rejects that, and no Spice dialect enables the option. The first two are filed as spiceai/spiceai#15220.

Are there any user-facing changes?

A federated join whose input is a scan with no projection now returns every column of that input, in the plan's order, from the remote engine; before, the remote either rejected the statement or returned the other input's columns in its place.

Review gate

• Adversarial review: skipped — on cd45aa3c7, codex exited 1: "Your workspace is out of credits. Ask your workspace owner to refill in order to continue." (receipt head=cd45aa3c7…, attempt=codex, engine=none). cd45aa3c7 only merges spiceai-55 into 3112eb390, and codex did review 3112eb390: job review-mvbmuoyc-w628yq, verdict needs-attention, 1 high finding. The finding is that a window over a join input is folded into the join's SELECT, so row_number() counts the joined rows, and the PR's window snapshots pin that placement. I reproduced it in SQLite and DuckDB: rn 1, 2 where the plan gives 1, 1. The base emits the same placement with the same values, and this PR changes only the column list, so it is out of scope here and filed as unparser: a window over a join input is evaluated over the joined rows (row_number() counts the join's output) spiceai#15239. Earlier rounds: review-mvak9j0j-i6js2l on 17aba5043 approved with no findings. review-mvak1zn3-w9ay8z on 6397f2b41 returned needs-attention with 1 high finding, fixed in 17aba5043. On 7e8cde350 and 34c21dda7 the review was skipped because codex was out of credits.
• /security-review: run by a read-only sub-agent over this round's diff, no security defects. Every q.* qualifier is a clone of the identifier already written in the FROM, with its quote style, and there is no new indexing or unwrap. It raised one low note, unverified, code inspection only: the collision check rescanned every relation for each item, which is cubic over a left-deep chain of bare joins. The check now counts each qualifier suffix once in a map. Earlier rounds: every emitted name goes through new_ident_quoted_if_needs. The two pre-existing shapes the 68-shape probe confirmed are filed as unparser: an Aggregate as a direct join input folds into the join's SELECT, and a mark join with no projection above drops its mark column spiceai#15220.
• /simplify: run by a read-only sub-agent; this round made 3 proposals. Two are applied: each item is now keyed once, and the comment on what an unset relation builder means is corrected. One is declined: moving the full_qualified_col rule next to emitted_qualifier would touch expr.rs for a single call site. The light checks were re-run green. Earlier rounds: 4 proposals, all applied in a9387e6e9.

Overlap: #261 and #247 edit other arms of plan.rs (union inputs; sort keys over volatile outputs), and #263 the Sort/Limit/Projection arms; this change touches the Join arm's item assembly and the Aggregate arm's list order.

[grokspice]

Follow-up for the two pre-existing shapes the review exposed, which this PR does not change beyond the empty list becoming a wildcard: spiceai/spiceai#15220 (an Aggregate directly as a join input folds into the join's own SELECT; a mark join with no projection above drops its mark column).

The 68-shape probe behind the "second probe" paragraph of the description, on 17aba5043, agrees with DuckDB 1.4.4 on 56 shapes. The 12 that do not, each with the base's behaviour for comparison:

shapes	this branch	base (SELECT FROM …, or the other side's items only)
Aggregate directly as a join input (3)	SELECT sum(t.b), t.a, u.* … GROUP BY t.a → binder error	accepted; 2 columns for a 3-column plan
mark join, bare or projected probe (5)	SELECT * FROM t → mark missing	parse error / mark missing
union or VALUES as a bare input (3)	refused (…every output of the input is qualified by a relation, but 'a' is not; Unsupported operator: Values)	the join silently dropped, or an internal error
three-part names under with_full_qualified_col(true) (3)	cat.sch.t.*, which DuckDB's parser rejects (no Spice dialect sets that option)	parse error / wrong columns

Everything else the probe covers — interleaved nested inputs, RIGHT/semi/anti joins, limits and filters over bare inputs, derived-table inputs, duplicate column names, self-joins, three-part names on the default and PostgreSQL dialects — matches the plan's rows and column count.

[Copilot]

🟡 Changes recommended

Unresolved wildcard handling can omit or duplicate output columns for aliased joins and window inputs.

2 open findings

• Window output leaves mixed-list wildcard unqualified · New
• Aliased joined inputs omit columns from wildcard projection · New

What changed in this PR

Corrects SQL unparser output so join columns and projection-less aggregate columns follow the logical plan’s schema.

Changes:

• Adds per-input join select lists and handles column-less inputs.
• Emits group keys before aggregate expressions.
• Adds regression tests and updates affected snapshots.

File	Description
datafusion/​sql/​tests/​cases/​plan_to_sql.rs	Adds join and aggregate regressions; updates snapshots.
datafusion/​sql/​src/​unparser/​plan.rs	Assembles join select lists and corrects aggregate column order.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Wildcard narrowing for a window over a derived input can reference a hidden table, producing invalid SQL.

1 open finding

• Use derived relation alias for window input wildcard expansion · New

2 resolved since last review

• Aliased joined inputs omit columns from wildcard projection
• Window output leaves mixed-list wildcard unqualified

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Two remaining window wildcard-expansion cases reference relations absent from the FROM clause, causing SQL binding failures.

0 open findings

2 resolved since last review

• Use derived relation alias for window input wildcard expansion
• Window output leaves mixed-list wildcard unqualified

Previously missed (2)

In code that hasn't changed since last review

Traverse filters to detect derived window inputs

datafusion/​sql/​src/​unparser/​plan.rs:1380

A filter above the window bypasses the derived-alias detection. For Inner(t, Filter(true, Window[rn](Projection[u.a](u))), ON true), the walk emits (SELECT u.a FROM u) AS derived_window_input, but this loop never enters because the input is a Filter. The schema fallback then replaces * with u.*, referencing a hidden table and causing a binding error. Traverse filters when locating the window's derived input, and add a filtered variant of the derived-window regression test.

Avoid wildcard expansion from qualified window output aliases

datafusion/​sql/​src/​unparser/​plan.rs:1382

This reads the window's output schema, including qualified outputs already represented by explicit SELECT items. A window expression using .alias_qualified(Some("x"), "rn") over bare u produces schema [u.a, x.rn]. Joined to bare t, the wildcard expands to u.*, x.* while retaining row_number() ... AS rn. Since x is not a FROM relation, the SQL fails to bind. Read the schema of the underlying input reached by the traversal instead, and add a regression test with a qualified window alias.

🧠 Review effort: Balanced

[spiceemma]

Both "Previously missed" findings from the review on 34c21dda7 reproduced and are fixed in 3509804 (whole_relation_items now walks through filters as well as windows, and reads the relations from the node it reaches rather than from the input's own schema).

1. Traverse filters to detect derived window inputs: new test test_a_filtered_window_over_a_derived_input_as_a_join_input_reads_the_derived_alias (Inner(t, Filter(true, Window[rn](Projection[u.a](u))), ON true)).

Red on 34c21dda7:

+SELECT t.*, u.*, row_number() OVER (ORDER BY derived_window_input.a ASC NULLS FIRST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS rn FROM t INNER JOIN (SELECT u.a FROM u) AS derived_window_input ON true QUALIFY true

DuckDB 1.5.5: Binder Error: Referenced table "u" not found! Candidate tables: "derived_window_input"

Green on 350980468:

SELECT t.*, derived_window_input.*, row_number() OVER (ORDER BY derived_window_input.a ASC NULLS FIRST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS rn FROM t INNER JOIN (SELECT u.a FROM u) AS derived_window_input ON true QUALIFY true
test cases::plan_to_sql::test_a_filtered_window_over_a_derived_input_as_a_join_input_reads_the_derived_alias ... ok

DuckDB binds it and returns 3 columns (a, a, rn), matching the plan's [t.a, u.a, rn].

2. Avoid wildcard expansion from qualified window output aliases: new test test_a_window_with_a_qualified_alias_as_a_join_input_reads_only_its_input_relations (row_number().alias_qualified(Some("x"), "rn") over bare u, joined to bare t).

Red on 34c21dda7:

+SELECT t.*, u.*, x.*, row_number() OVER (ORDER BY u.a ASC NULLS FIRST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS rn FROM t INNER JOIN u ON true

DuckDB 1.5.5: Binder Error: Referenced table "x" not found! Candidate tables: "t"

Green on 350980468:

SELECT t.*, u.*, row_number() OVER (ORDER BY u.a ASC NULLS FIRST RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS rn FROM t INNER JOIN u ON true
test cases::plan_to_sql::test_a_window_with_a_qualified_alias_as_a_join_input_reads_only_its_input_relations ... ok

DuckDB binds it and returns 4 columns (a, a, b, rn), matching the plan's [t.a, u.a, u.b, x.rn].

Checks on 350980468

• cargo test -p datafusion-sql --test sql_integration -- plan_to_sql: 424 passed; 22 failed. The 22 failing names are identical (diff empty) to the base spiceai-55 at cf084aa85, run in a separate worktree: 409 passed; 22 failed.
• cargo test -p datafusion-sql --lib: 103 passed.
• cargo fmt --all -- --check: clean. cargo +1.97.1 clippy -p datafusion-sql --all-targets -- -D warnings: clean.

Seen while verifying, already on the base and not changed here. When a window over a join input is folded into the join's SELECT, row_number() is computed over the joined rows instead of over the window's input. With t = 2 rows and u = 3 rows, both emitted statements above return rn 1..6 in DuckDB. A hand-written statement for the plan (SELECT t.a, w.a, w.rn FROM t, (SELECT a, row_number() OVER (ORDER BY a) rn FROM u) w) returns 1..3, each value twice. The existing test_a_window_over_a_bare_scan_as_a_join_input_narrows_its_wildcard snapshot has the same placement, and so does the base's SELECT *, row_number() … FROM t INNER JOIN u ON true. This PR only fixes the column list, so I'm reporting it here and not changing it.

[Copilot]

🟡 Changes recommended

Wildcard resolution through Sort and Distinct wrappers can still emit invalid table references.

1 open finding

• Resolve wildcards through folded Sort and Distinct wrappers · New

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Nested semi/anti joins containing windows can still emit wildcard qualifiers absent from the FROM clause, causing binding failures.

1 open finding

• Semi/anti joins incorrectly forward probe window projections · New

1 resolved since last review

• Resolve wildcards through folded Sort and Distinct wrappers

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Unresolved wildcard-scope and qualifier-collision cases can produce invalid SQL or duplicate output columns.

3 open findings

• Empty window incorrectly emits hidden input qualifier · New
• No-op limit prevents derived window scope propagation · New
• Synthesized wildcard qualifiers collide across FROM sources · New

1 resolved since last review

• Semi/anti joins incorrectly forward probe window projections

🧠 Review effort: Balanced

[spiceemma]

Escalated: how to handle two FROM relations that emit the same t.* qualifier (thread)

This reproduces on 0800a1231. It is a regression against the base, and fixing it means picking a behaviour, so I've left it for a maintainer to decide rather than choosing one myself.

Plan: Inner(SubqueryAlias(t, main.l[a]), other.t[b], ON true), both inputs unprojected. emitted_qualifier drops the schema unless full_qualified_col is set, so both relations come out as t:

	emitted SQL	DuckDB 1.5.5	SQLite 3 (other attached)
base spiceai-55 (cf084aa85)	SELECT * FROM "main".l AS t INNER JOIN "other".t ON true	(11, 22), 2 columns ✓	11,22 ✓
this PR (0800a1231), default dialect	SELECT t.*, t.* FROM "main".l AS t INNER JOIN "other".t ON true	Binder Error: Ambiguous reference to table "t" (use: .t or other.t)	—
this PR, SqliteDialect	SELECT `t`.*, `t`.* FROM `main`.`l` AS `t` INNER JOIN `other`.`t` ON true	—	11,22,11,22, 4 columns for a 2-column plan

Spelling out the full name doesn't help either. SELECT t.*, other.t.* … is a parser error in both DuckDB (syntax error at or near "*") and SQLite (near "*": syntax error).

Options I can see:

1. Refuse (not_impl_err) when two relations in the join's FROM emit the same qualifier. This fits the PR's "refuse rather than guess" stance, but it gives up a pushdown that the base unparsed correctly.
2. Fall back to a bare * when the qualifiers collide and the two reasons the PR keeps t.* don't apply (no USING and no column-less input). This brings back the base output for this shape, but adds another case to the item assembly.
3. Re-alias the colliding relation in the FROM (e.g. "other".t AS t_1) and qualify its wildcard by the new alias. This is the most complete fix, but it reaches into relation building and every reference to that input (ON, filters, the list).

A note for whoever picks this up: the last four review rounds each found another operator that folds into the window's SELECT. This run added Filter, Sort, Distinct::All, semi/anti probe, no-op Limit and empty-window handling to wildcard_source. Deriving the narrowed relations from what the input's walk actually put into the FROM, instead of from the plan's shape, would close that class of finding and would also see collisions like this one. That's a larger change than this run should make on its own, so it's included here as part of the decision.

Commits from this run: 350980468 (filter / qualified window alias), 8869f03d7 (sort / DISTINCT), 6c483123b (semi/anti probe), 0800a1231 (no-op limit / empty window). Each has its red→green test output in its thread or in the comment above. On every head the plan_to_sql suite fails only the base's 22 tests.

[Copilot]

🔵 Needs a closer look

Scope-sensitive wildcard reconstruction needs human validation, and an unresolved wildcard representation can still produce incorrect columns.

2 open findings

• Handle unqualified wildcard projections to prevent incorrect column counts · New
• Synthesized wildcard qualifiers collide across FROM sources

2 resolved since last review

• No-op limit prevents derived window scope propagation
• Empty window incorrectly emits hidden input qualifier

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Wildcard binding across nested joins, derived tables, and SQL dialects warrants final human validation.

2 open issues

• Handle unqualified wildcard projections to prevent incorrect column counts
• Synthesized wildcard qualifiers collide across FROM sources

_{Review effort: Balanced}

[grokspice]

Wildcard-qualifier hold released: both open threads are fixed in 3112eb390 (head cd45aa3c7)

The user asked for both open review threads to be fixed and resolved, which releases the hold from the escalation.

Approach: structural. Each join input's * is now narrowed to the relations its walk actually wrote into the FROM. Those are the RelationBuilder plus the joins it appended to the shared FROM since a snapshot taken before the left input's walk, read under the names the FROM gives them. The plan's schema qualifiers are no longer used. I chose this over a fallback bolted onto the plan-shape reading for three reasons:

• It replaces wildcard_source/whole_relation_items and the operator list they walked (Window, Filter, Sort, Distinct::All, no-op Limit, semi/anti probe). That closes the class the last four rounds kept finding: each round turned up one more operator that folds into the window's SELECT.
• The collision check needs the FROM's relation names anyway.
• The plan-shape reading had one more silent regression that only the FROM shows. SubqueryAlias(j, t ⋈ u) ⋈ w emits FROM t AS j INNER JOIN u ... (the alias renames only t). 0800a1231 listed j.*, w.*, 2 columns for a 3-column plan in SQLite and DuckDB. The base's SELECT * was correct, and this branch lists j.*, u.*, w.*.

Size: plan.rs + ast.rs are +343/−156 lines (net +187; +153 net code lines, the rest doc comments). All 18 earlier PR tests pass unchanged, with no snapshot re-pinned.

Collision decision as implemented. join_select_list checks each q.* in a join's list against the relations of its FROM:

• Bare * fallback. When a q.* matches more than one relation, the list is exactly one q.* per relation in FROM order, and no join merges columns (USING/NATURAL), the join emits a bare *. That restores the base's output.
• Refusal. Any other list with a colliding q.* gets not_impl_err!. That covers USING, a column-less input, and an input that lists its own columns; the base returned wrong column counts for all three.
• Re-aliasing is not done here. The same ambiguity hits ON/WHERE column references on the base already, shown by a run in unparser: two FROM relations that emit the same qualifier make every reference qualified by it ambiguous (main.l AS t beside other.t) spiceai#15238.

Thread 2. SelectItem::UnnamedExpr(Expr::Wildcard) reproduced: SELECT t.*, * ... returned 3 columns for a 2-column plan. It is now treated as an unqualified * in both singleton and mixed lists and narrowed to the projection's FROM relations.

Also refused, not to regress: a join input whose output still carries a mark join's mark. Read through * it would lose the mark silently (spiceai/spiceai#15220). 0800a1231 emitted a dangling u.*, which both engines reject.

Evidence (red on 0800a1231, green on 3112eb390, details in each thread):

• New tests: test_join_inputs_answering_to_one_qualifier_read_through_a_bare_wildcard, ..._are_refused_where_a_bare_wildcard_misreads, test_an_unqualified_wildcard_expr_in_a_join_input_reads_only_that_input, test_an_alias_over_a_join_as_a_join_input_reads_every_relation_it_holds, test_a_mark_join_read_through_a_wildcard_as_a_join_input_is_refused. All 5 failed on 0800a1231 and pass here.
• Probe of 17 shapes on the default dialect and SqliteDialect, each statement run in SQLite 3.50.4 and DuckDB 1.4.4 against a hand-written statement for the plan. Disagreeing engine runs: base e8d8abae9 24, 0800a1231 36 (first 15 shapes), 3112eb390 6. The 6 are the two ON t.a = other.t.a shapes, where the base emits the identical rejected statement (unparser: two FROM relations that emit the same qualifier make every reference qualified by it ambiguous (main.l AS t beside other.t) spiceai#15238).
• cargo test -p datafusion-sql --test sql_integration: 851 passed; 22 failed on the head cd45aa3c7 (3112eb390 merged with spiceai-55, which had gone CONFLICTING after fix(unparser): keep a union input's own ORDER BY and LIMIT inside that input #261 landed; the only conflict was the two sides' tests appended at the end of plan_to_sql.rs, and both sets are kept). Base ddd5c8ce6 gives 828 passed; 22 failed. The 22 failing names are identical; I compared them as sets, and none is introduced here. Before the merge, 3112eb390 gave 838 / 22 against 815 / 22 on e8d8abae9.
• On cd45aa3c7: --lib gives 103 passed, and cargo fmt --all -- --check and cargo clippy -p datafusion-sql --all-targets -- -D warnings are clean.

Review gate: codex reviewed 3112eb390 (job review-mvbmuoyc-w628yq) and returned needs-attention with 1 high finding. The finding: a window over a join input is folded into the join's SELECT, so row_number() counts the joined rows. I reproduced it on the base with identical values, so it predates this PR, and filed it as spiceai/spiceai#15239. On cd45aa3c7, codex exited 1 (out of credits), and the PR body's ## Review gate block declares that as a skip. A read-only security pass found no defects, and its one low note (a cubic rescan in the collision check) is addressed. Two of three simplify proposals are applied.

Process note: the shared build directory let a peer worktree's test binary run under this worktree's name. Every count above comes from a binary that I confirmed was built from this worktree's sources.

[Copilot]

🔵 Needs a closer look

Nested join scopes and dialect-specific wildcard collision handling require final human validation of output-schema preservation.

No open issues 🎉

2 resolved since last review

• Handle unqualified wildcard projections to prevent incorrect column counts
• Synthesized wildcard qualifiers collide across FROM sources

_{Review effort: Balanced}

[grokspice]

Merged spiceai-55 (0e01a882f, which brings in #246 and #247) as e1c02e750. The PR had become DIRTY once both of those merged.

Conflicts. There were 8 hunks.

• plan.rs, 1 hunk. Append/append: this PR's FromRelations and wildcard helpers next to fix(unparser): keep a qualifier that an UNNEST or FLATTEN alias, a schema-qualified table or a struct field path binds (refs spiceai/spiceai#13442) #246's collect_table_name_idents. I kept both.
• plan_to_sql.rs, 7 hunks. All append/append, where each side added whole tests at the same spot. I kept both sides.

Adjustments the merge needed beyond the textual conflicts.

• Four fully qualified paths became unused_qualifications errors, because the base now imports HashMap, Projection and get_field: std::collections::HashMap ×2 and datafusion_expr::logical_plan::Projection::try_new_with_schema ×2. I shortened them.
• fix(unparser): keep a qualifier that an UNNEST or FLATTEN alias, a schema-qualified table or a struct field path binds (refs spiceai/spiceai#13442) #246's new test test_derived_aggregate_keeps_a_sibling_relation_whose_name_extends_an_enclosed_one asserted the old aggregate select order, SELECT count(1) AS m, t.s.a …. This PR deliberately emits group keys first, in the plan schema's order, so the merged tree emits SELECT t.s.a, count(1) AS m …. Apart from that order, the merged SQL is byte-identical to what spiceai-55 emits for the same plan; I printed both. I updated the assertion to the new order.

Re-validation of the merged tree.

cargo clippy -p datafusion-sql --all-targets --all-features -- -D warnings   -> Finished, no warnings
cargo test -p datafusion-sql                                                 -> unittests 104 passed; sql_integration 942 passed, 0 failed; doctests 0 failed, 1 ignored
git diff --stat 0e01a882f e1c02e750                                          -> 3 files, +1225 −39 (before the merge: 3 files, +1229 −38 against ddd5c8ce6)

Not changed here. spiceai-55 already emits … AS derived_aggregate_1 INNER JOIN t.s ON t.a = t.s.a for that test's plan, reading t.a through a derived table that hides t. That is unverified, code inspection only: I didn't run the SQL against an engine. This merge leaves it as it is.

[Copilot]

🔵 Needs a closer look

Dialect-dependent wildcard expansion and shared SELECT state across nested joins warrant final human validation.

No open issues 🎉

_{Review effort: Balanced}