fix(unparser): inline a predicate's reference to an unnamed projection output

[grokspice]

Summary

When a Filter references a Projection output that the projection does not name, the unparser emitted the column's logical name as a quoted identifier:

SELECT (t.a + t.b) FROM t WHERE ("t.a + t.b" > 1)

"t.a + t.b" is a single quoted identifier. t has no such column and the SELECT list introduces none, so PostgreSQL answers 42703 and DuckDB a Binder Error. Federated pushdown of a filter over a computed column failed at the remote engine rather than at plan time.

Root cause

The LogicalPlan::Filter arm of select_to_sql_recursively has an "unproject" step for the aggregate (HAVING) and window (QUALIFY) paths, but the plain WHERE path unparsed filter.predicate verbatim. A column referring to a projection output carries that output's logical name, which is a description of the expression rather than an identifier the emitted statement carries.

Changes

• find_projection_node_within_select — locates the Projection that will be flattened into the same SELECT, mirroring the existing find_agg_node_within_select. Returns None once the SELECT list is taken, since a projection reached after that becomes a derived table whose columns are addressable from outside.
• unproject_unnamed_projection_exprs — replaces a reference to an unnamed projection output with the expression that produces it, so the predicate needs no name at all:

SELECT (t.a + t.b) FROM t WHERE ((t.a + t.b) > 1)

Deliberately left unchanged:

• an aliased output (… AS s) and a bare column — both already carry a name the emitted SELECT exposes, so a reference to one binds as written;
• a volatile expression — inlining would evaluate it a second time, in a clause that can see a different value than the SELECT list did. That trades a loud unbindable reference for silently wrong rows, so the loud failure is kept.

Test plan

Five tests in datafusion/sql/tests/cases/plan_to_sql.rs:

test	covers
test_filter_on_unnamed_projection_output_binds	the reported regression — fails before this change, passes after
test_filter_on_named_projection_output_is_unchanged	alias and bare-column outputs are not inlined
test_filter_on_unnamed_projection_output_used_twice	every reference is inlined, not just the first
test_stacked_filters_on_unnamed_projection_output	stacked filters collapsing into one WHERE
test_filter_on_unnamed_volatile_projection_output_is_not_inlined	the volatility guard

cargo test -p datafusion-sql — 505 passed, 22 failed, against a 500 passed, 22 failed baseline on spiceai-54 at edd8861e6. The 22 failures are identical in both runs (verified by diffing the failing-test sets) and pre-date this change; this PR adds 5 passing tests and regresses none.

cargo clippy -p datafusion-sql --all-targets and cargo fmt --all -- --check report nothing in the added code. Note that spiceai-54 is not currently rustfmt-clean, so cargo fmt --all rewrites unrelated files; those were reverted to keep this diff in scope — including two hunks in unproject_sort_expr that PR #198 also touches.

[Copilot]

Pull request overview

Fixes SQL unparsing for filters referencing unnamed computed projection outputs.

Changes:

• Locates flattened projections and inlines unnamed deterministic expressions.
• Preserves volatile expressions.
• Adds regression snapshots for projection-filter scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
datafusion/sql/src/unparser/utils.rs	Adds projection lookup and expression substitution helpers.
datafusion/sql/src/unparser/plan.rs	Applies substitution while generating WHERE.
datafusion/sql/tests/cases/plan_to_sql.rs	Adds regression tests and formatting cleanup.

Suppressed comments (2)

datafusion/sql/src/unparser/utils.rs:339

• SELECT aliases are not visible to WHERE in PostgreSQL or DuckDB. The new aliased case therefore still emits an unbindable WHERE (s > 1) (or can silently bind to an unrelated source column named s). Alias outputs must be replaced with the alias's inner expression here, just like unnamed computed outputs, and the test expectation should use WHERE ((t.a + t.b) > 1).

    if matches!(expr, Expr::Alias(_) | Expr::Column(_)) || expr.is_volatile() {

datafusion/sql/src/unparser/utils.rs:295

• This stops at any node other than another Filter, although some such nodes are flattened into the same SelectBuilder. For example, Filter -> Sort(fetch=None) -> Projection(a + b) passes through the Sort arm without deriving, so this returns None and the predicate is still emitted as the unbindable quoted logical name. Continue through same-select transparent nodes (at least a non-fetch Sort) while retaining boundaries for operators such as fetching sorts/limits.

        // Stacked filters collapse into one `WHERE`, so keep looking through them.
        LogicalPlan::Filter(_) => {
            find_projection_node_within_select(input, already_projected)
        }
        _ => None,

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.