fix: one inlining policy for the rules that inline a projection column - #25456

…ards (apache#25452)

## Which issue does this PR close?

- Part of apache#25451.

This PR closes 5 of the 14 real test gaps that the mutation run found.
It does not close the issue.

## Rationale for this change

`extract_leaf_expressions.rs` holds several guards that stop a rewrite
in one specific case. Each guard went in with a bug fix, and each fix
came with a plan snapshot test. A plan snapshot test asserts the shape
of one output plan. It does not assert that the guard makes its decision
for the reason the guard documents.

A `cargo-mutants` run over the file shows the result. Three guards let a
mutation through with no test failure:

| Function | Mutation that survived | Effect if this happened for real |
|---|---|---|
| `passthrough_column` | guard `col.name == alias.name` to `true` | A
renaming alias (`test.a AS b`) counts as a pass-through column. The
recovery projection that restores the name is then dropped, and the
rename is lost. |
| `is_pure_extraction_projection` | body to `true`, and the
`EXTRACTED_EXPR_PREFIX` guard to `true` | A projection that holds a
`__common_expr_1` alias is pushed down a second time. The expression is
re-extracted under a new alias that the parent cannot resolve. |
| `merge_would_duplicate_kept_expr` | `>` to `>=` | The guard fires on a
single reference site. Every merge into a child projection is then
blocked, and leaf extraction stops working over a projection. |
| `merge_would_duplicate_kept_expr` | `&&` to `\|\|` | Plain columns
count as reference sites. A merge that touches only plain columns is
blocked, although a plain column is cheap to duplicate. |

Each of these is a silent behaviour change. The suite stays green.

## What changes are included in this PR?

Three unit tests, one per function, in the existing `tests` module of
`datafusion/optimizer/src/extract_leaf_expressions.rs`. Each test calls
the guard directly and asserts both answers: the case where the guard
must fire, and the case where it must not. The file already uses this
style for `find_owning_input`.

- `test_passthrough_column_rejects_renaming_alias` covers a bare column,
a trivial rename that keeps the name, a rename that changes the name,
and an alias over a non-column expression.
- `test_is_pure_extraction_projection` covers a plan that is not a
projection, a columns-only projection, an extraction projection, a
projection that uses a `CommonSubexprEliminate` alias, and a projection
with a bare expression.
- `test_merge_would_duplicate_kept_expr_counts_only_kept_columns` covers
one reference site to a `KeepInPlace` column, two sites, one site plus a
pass-through reference, and two sites that reference a plain column
only. The pass-through case is the one that
apache#23655 reported.

The PR adds a small `keep_in_place_udf` test helper. It gives a
non-volatile `KeepInPlace` expression, which the existing `volatile_udf`
helper cannot.

There is no change to product code.

## What is the testing strategy for this PR?

This PR is tests only.

`cargo test --profile ci -p datafusion-optimizer` goes from 882 to 885
library tests. The 26 integration tests and 5 documentation tests are
unchanged. All pass.

To show that the new tests close the gaps, re-run `cargo-mutants` on the
three functions:

```bash
cargo mutants -p datafusion-optimizer \
  -f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
  -F 'passthrough_column|is_pure_extraction_projection|merge_would_duplicate_kept_expr' \
  --profile ci --timeout 600 --jobs 2
```

| | Before | After |
|---|---|---|
| Caught | 16 | 21 |
| Missed | 5 | 0 |
| Unviable | 1 | 1 |

## Are there any user-facing changes?

No.

Note: the test for `merge_would_duplicate_kept_expr` becomes dead when
apache#25456 lands, because that PR
deletes the function. Whichever merges second drops that one test; the
other two tests apply unchanged.

Part of the leaf-pushdown EPIC:
apache#25459

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <[email protected]>

…ards (apache#25452)

## Which issue does this PR close?

- Part of apache#25451.

This PR closes 5 of the 14 real test gaps that the mutation run found.
It does not close the issue.

## Rationale for this change

`extract_leaf_expressions.rs` holds several guards that stop a rewrite
in one specific case. Each guard went in with a bug fix, and each fix
came with a plan snapshot test. A plan snapshot test asserts the shape
of one output plan. It does not assert that the guard makes its decision
for the reason the guard documents.

A `cargo-mutants` run over the file shows the result. Three guards let a
mutation through with no test failure:

| Function | Mutation that survived | Effect if this happened for real |
|---|---|---|
| `passthrough_column` | guard `col.name == alias.name` to `true` | A
renaming alias (`test.a AS b`) counts as a pass-through column. The
recovery projection that restores the name is then dropped, and the
rename is lost. |
| `is_pure_extraction_projection` | body to `true`, and the
`EXTRACTED_EXPR_PREFIX` guard to `true` | A projection that holds a
`__common_expr_1` alias is pushed down a second time. The expression is
re-extracted under a new alias that the parent cannot resolve. |
| `merge_would_duplicate_kept_expr` | `>` to `>=` | The guard fires on a
single reference site. Every merge into a child projection is then
blocked, and leaf extraction stops working over a projection. |
| `merge_would_duplicate_kept_expr` | `&&` to `\|\|` | Plain columns
count as reference sites. A merge that touches only plain columns is
blocked, although a plain column is cheap to duplicate. |

Each of these is a silent behaviour change. The suite stays green.

## What changes are included in this PR?

Three unit tests, one per function, in the existing `tests` module of
`datafusion/optimizer/src/extract_leaf_expressions.rs`. Each test calls
the guard directly and asserts both answers: the case where the guard
must fire, and the case where it must not. The file already uses this
style for `find_owning_input`.

- `test_passthrough_column_rejects_renaming_alias` covers a bare column,
a trivial rename that keeps the name, a rename that changes the name,
and an alias over a non-column expression.
- `test_is_pure_extraction_projection` covers a plan that is not a
projection, a columns-only projection, an extraction projection, a
projection that uses a `CommonSubexprEliminate` alias, and a projection
with a bare expression.
- `test_merge_would_duplicate_kept_expr_counts_only_kept_columns` covers
one reference site to a `KeepInPlace` column, two sites, one site plus a
pass-through reference, and two sites that reference a plain column
only. The pass-through case is the one that
apache#23655 reported.

The PR adds a small `keep_in_place_udf` test helper. It gives a
non-volatile `KeepInPlace` expression, which the existing `volatile_udf`
helper cannot.

There is no change to product code.

## What is the testing strategy for this PR?

This PR is tests only.

`cargo test --profile ci -p datafusion-optimizer` goes from 882 to 885
library tests. The 26 integration tests and 5 documentation tests are
unchanged. All pass.

To show that the new tests close the gaps, re-run `cargo-mutants` on the
three functions:

```bash
cargo mutants -p datafusion-optimizer \
  -f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
  -F 'passthrough_column|is_pure_extraction_projection|merge_would_duplicate_kept_expr' \
  --profile ci --timeout 600 --jobs 2
```

| | Before | After |
|---|---|---|
| Caught | 16 | 21 |
| Missed | 5 | 0 |
| Unviable | 1 | 1 |

## Are there any user-facing changes?

No.

Note: the test for `merge_would_duplicate_kept_expr` becomes dead when
apache#25456 lands, because that PR
deletes the function. Whichever merges second drops that one test; the
other two tests apply unchanged.

Part of the leaf-pushdown EPIC:
apache#25459

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <[email protected]>

…25478)

## Which issue does this PR close?

- Closes apache#25446.

Part of the leaf-pushdown EPIC:
apache#25459

## Rationale for this change

A valid statement fails to plan. A sub-query projection renames a column
to the name of a different column of the same input, and the query reads
a struct field above it.
`datafusion.optimizer.enable_leaf_expression_pushdown` is `true` by
default, so this is a plain planning failure.

```sql
create table t(a int, b int, s struct<x varchar>) as values (1, 10, {x: 'p'}), (2, 20, {x: 'q'});

-- 1. a rename beside a same-name alias, under a filter
select b, a, s['x'] from (select t.a as b, t.b as a, s from t) where a > 0;

-- 2. the same shape under a limit
select b, a, s['x'] from (select t.a as b, t.b as a, s from t) limit 10;

-- 3. a swap of two column names
select a, c, s['x'] from (select t.b as a, t.a as c, s from t) where a > 0;
```

Observed on `main`, for all three:

```text
Optimizer rule 'push_down_leaf_projections' failed
caused by
Schema error: Schema contains qualified field name t.a and unqualified field name a which would be ambiguous
```

Expected, and what you get with `set
datafusion.optimizer.enable_leaf_expression_pushdown = false;`:

```text
+---+----+--------+        +----+---+--------+
| b | a  | t.s[x] |        | a  | c | t.s[x] |
+---+----+--------+        +----+---+--------+
| 1 | 10 | p      |        | 10 | 1 | p      |
| 2 | 20 | q      |        | 20 | 2 | q      |
+---+----+--------+        +----+---+--------+
   statements 1 and 2         statement 3
```

`order by` and `group by` above the rename fail the same way. Query
generators emit this shape.

## What changes are included in this PR?

Both changes are in
`datafusion/optimizer/src/extract_leaf_expressions.rs`.

**1. The merge of the extraction projection.**
`build_extraction_projection_impl` merges the extraction projection into
the projection below it. For every column the parent needs, it resolved
the name through the projection's rename map and then added the *input*
column that the name resolves to. For `select t.a as b, t.b as a, s from
t` the parent's `b` resolves to `t.a`, and `t.a` lands beside the output
field `a` of the other rename. `Projection::try_new` rejects that
schema.

The merge keeps every expression of the projection below it, so the
parent can still read each name that projection produces. Those names
are now skipped instead of resolved, and a pass-through column whose
name is one of the projection's output names is never added. A rename is
a computed output, and the parent refers to it by its output name only.

**2. The recovery check.** With the merge fixed, the shape reaches
`split_and_push_projection`, which decided if it needs a recovery
projection from the set of unqualified field names alone. Equal name
sets do not prove equal values: the pushed plan exposes the table column
`t.a` where the projection computed `t.a AS b`. Without this hunk the
statements above return the columns swapped instead of an error. The
recovery projection now also stays when a recovery expression is not a
pass-through of a column.

That second hunk is the same change as
apache#25445, down to the text, so
the two merge without a conflict. If that PR lands first, the hunk falls
out of the rebase.

**How this composes with the other open PRs on this function.**
apache#25412 resolves both sides of
the pass-through comparison against the input schema, and
apache#25456 replaces the rename map
with `ProjectionInliner`. This change sits above both: it decides which
columns to add before either one resolves a name. I merged this branch
into a local branch that holds both PRs. The single conflict is the loop
header, which takes the guards of this PR around the body of the other
two. The optimizer tests, the `struct`, `cse` and `push_down_filter`
sqllogictest files, and the differential fuzz all pass there with the
swap-alias shape of the fuzz generator turned on: 250 short cases, 125
short Parquet cases, 5000 extended cases and 1000 extended Parquet
cases, 0 failures and 0 skips.

## What is the testing strategy for this PR?

- `datafusion/sqllogictest/test_files/struct.slt`: the three statements
above, plus the `order by` and `group by` variants, plus a swap where
the struct field name is also a column name of the table, plus one
`EXPLAIN` that shows the renames above the extraction projection and the
struct field still read at the scan.
- `datafusion/optimizer/src/extract_leaf_expressions.rs`:
`test_extract_above_projection_that_swaps_column_names` covers the
ambiguous schema.
`test_extract_above_projection_that_redefines_column_name` covers the
recovery projection that must keep a rename alive.
- Three plan snapshots of neighbouring tests lose a duplicate
pass-through column in the intermediate stage. The optimized plans of
those three tests do not change.

Commands and results:

```text
cargo test --profile ci -p datafusion-optimizer
  884, 26, 5 passed; 0 failed

cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests
  520/520 files completed, 0 failures

cargo clippy --profile ci -p datafusion-optimizer --all-targets -- -D warnings
  clean
```

## Are there any user-facing changes?

Statements of this shape plan and run. There is no API change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <[email protected]>

Co-Authored-By: Claude Opus 5.5 <[email protected]>

# Conflicts:
#	datafusion/optimizer/src/push_down_filter.rs

… PushDownFilter

Rebuilt on the new apache#25456, where PushDownFilter is unchanged from main.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

…he#27152)

## Which issue does this PR close?

- Part of apache#27131.

## Rationale for this change

A `MoveTowardsLeafNodes` expression loses leaf pushdown as soon as it
shares a
projection with a `KeepInPlace` expression and a `RepartitionExec` is
inserted.
The two symptom classes are:

```sql
-- errors: "file_row_index() is source dependent and cannot be evaluated directly"
SELECT file_row_index(), id + 1 FROM t;

-- no error, but reads the whole struct instead of pruning to one field
SELECT s['value'], id + 1 FROM t;
```

`ProjectionExec::benefits_from_input_partitioning` is true as soon as
*any*
expression in the projection is `KeepInPlace` (the default placement).
That one
flag does two opposing things:

- `EnsureRequirements` inserts a `RepartitionExec` beneath the
projection, and
- `RepartitionExec::try_swapping_with_projection` then refuses to let
that same
  projection descend past it.

So the leaf expression can never reach the source. The leaf call does
not have to
be wrapped in anything — an unrelated computed column in the same
projection is
enough, because the flag is a property of the whole projection, not of
one
expression. `get_field` is the headline case: nested column pruning is
its
documented purpose and it degrades silently.

The trigger needs a `RepartitionExec`, so the table has to exceed
`batch_size`
(`EnsureRequirements` uses `batch_size` as a row-count threshold). That
is why
this has not shown up in the existing small-fixture tests.

## What changes are included in this PR?

`ProjectionPushdown` now splits such a projection instead of giving up:

- the leaf expressions move into a lower projection that the source
absorbs;
- the remaining computation stays above the repartition, where it is
still
  parallelised.

Both guards keep their intent rather than one being traded for the
other: the
scan only gains the cheap leaf expression, and the expensive computation
is still
run in parallel.

Deliberate limits:

- The split is committed only when `child.try_swapping_with_projection`
actually
returns `Some` for the lower projection, i.e. when the source really
does
  absorb it. Otherwise the plan is left untouched.
- Only `Partitioning::RoundRobinBatch` is handled. Hash and range
partitioning
reference columns by index, so changing the schema below them would
require
  remapping their expressions; that is left out of this PR.
- Only the columns the outer expressions still reference outside the
extracted
leaves are passed through, so `s['a'] + 1` does not end up reading the
whole
  struct alongside the extracted field.

## What is the testing strategy for this PR?

- `datafusion/sqllogictest/test_files/file_row_index.slt` — value-based
regression tests for the error class: bare marker plus a computed
sibling,
`+ 1`, `arrow_typeof`, two different markers mixed, a duplicated marker,
and
  non-regression for `Filter` / `Sort` / `Aggregate`.
- `datafusion/sqllogictest/test_files/projection_pushdown.slt` — plan
assertions
for the `get_field` class. Value comparison can never catch that one:
without
pruning the results are still correct, only more data is read, so the
test has
  to assert the pruned `projection=[...]` in the plan.
- `datafusion/physical-optimizer` unit tests: 50 passed.

Two traps are called out in comments in both test files, because either
one makes
the tests pass against unfixed code:

1. the fixture must exceed `batch_size`, or no `RepartitionExec` is
inserted and
   there is nothing to reproduce;
2. a bare `LIMIT n` is pushed into the scan and hides the bug again.

Verified by reverting the source file with `git checkout main --
<file>`: the new
tests then fail with 6 errors and 16 plan-diff lines.

## Are there any user-facing changes?

No API changes. Queries that previously failed with "is source dependent
and
cannot be evaluated directly" now succeed, and `get_field` regains
nested column
pruning when it shares a projection with other expressions.

One plan-shape note for apache#25456 (inlining projection columns), which
touches
adjacent code: its test pins `file_row_index()` used twice as *not*
inlined. When
the split here fires, the two identical leaf calls are de-duplicated
into a
single lower-projection column. Values are unaffected; only the plan
shape
differs. Happy to rebase in whichever order is convenient.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <[email protected]>

…25478)

## Which issue does this PR close?

- Closes apache#25446.

Part of the leaf-pushdown EPIC:
apache#25459

## Rationale for this change

A valid statement fails to plan. A sub-query projection renames a column
to the name of a different column of the same input, and the query reads
a struct field above it.
`datafusion.optimizer.enable_leaf_expression_pushdown` is `true` by
default, so this is a plain planning failure.

```sql
create table t(a int, b int, s struct<x varchar>) as values (1, 10, {x: 'p'}), (2, 20, {x: 'q'});

-- 1. a rename beside a same-name alias, under a filter
select b, a, s['x'] from (select t.a as b, t.b as a, s from t) where a > 0;

-- 2. the same shape under a limit
select b, a, s['x'] from (select t.a as b, t.b as a, s from t) limit 10;

-- 3. a swap of two column names
select a, c, s['x'] from (select t.b as a, t.a as c, s from t) where a > 0;
```

Observed on `main`, for all three:

```text
Optimizer rule 'push_down_leaf_projections' failed
caused by
Schema error: Schema contains qualified field name t.a and unqualified field name a which would be ambiguous
```

Expected, and what you get with `set
datafusion.optimizer.enable_leaf_expression_pushdown = false;`:

```text
+---+----+--------+        +----+---+--------+
| b | a  | t.s[x] |        | a  | c | t.s[x] |
+---+----+--------+        +----+---+--------+
| 1 | 10 | p      |        | 10 | 1 | p      |
| 2 | 20 | q      |        | 20 | 2 | q      |
+---+----+--------+        +----+---+--------+
   statements 1 and 2         statement 3
```

`order by` and `group by` above the rename fail the same way. Query
generators emit this shape.

## What changes are included in this PR?

Both changes are in
`datafusion/optimizer/src/extract_leaf_expressions.rs`.

**1. The merge of the extraction projection.**
`build_extraction_projection_impl` merges the extraction projection into
the projection below it. For every column the parent needs, it resolved
the name through the projection's rename map and then added the *input*
column that the name resolves to. For `select t.a as b, t.b as a, s from
t` the parent's `b` resolves to `t.a`, and `t.a` lands beside the output
field `a` of the other rename. `Projection::try_new` rejects that
schema.

The merge keeps every expression of the projection below it, so the
parent can still read each name that projection produces. Those names
are now skipped instead of resolved, and a pass-through column whose
name is one of the projection's output names is never added. A rename is
a computed output, and the parent refers to it by its output name only.

**2. The recovery check.** With the merge fixed, the shape reaches
`split_and_push_projection`, which decided if it needs a recovery
projection from the set of unqualified field names alone. Equal name
sets do not prove equal values: the pushed plan exposes the table column
`t.a` where the projection computed `t.a AS b`. Without this hunk the
statements above return the columns swapped instead of an error. The
recovery projection now also stays when a recovery expression is not a
pass-through of a column.

That second hunk is the same change as
apache#25445, down to the text, so
the two merge without a conflict. If that PR lands first, the hunk falls
out of the rebase.

**How this composes with the other open PRs on this function.**
apache#25412 resolves both sides of
the pass-through comparison against the input schema, and
apache#25456 replaces the rename map
with `ProjectionInliner`. This change sits above both: it decides which
columns to add before either one resolves a name. I merged this branch
into a local branch that holds both PRs. The single conflict is the loop
header, which takes the guards of this PR around the body of the other
two. The optimizer tests, the `struct`, `cse` and `push_down_filter`
sqllogictest files, and the differential fuzz all pass there with the
swap-alias shape of the fuzz generator turned on: 250 short cases, 125
short Parquet cases, 5000 extended cases and 1000 extended Parquet
cases, 0 failures and 0 skips.

## What is the testing strategy for this PR?

- `datafusion/sqllogictest/test_files/struct.slt`: the three statements
above, plus the `order by` and `group by` variants, plus a swap where
the struct field name is also a column name of the table, plus one
`EXPLAIN` that shows the renames above the extraction projection and the
struct field still read at the scan.
- `datafusion/optimizer/src/extract_leaf_expressions.rs`:
`test_extract_above_projection_that_swaps_column_names` covers the
ambiguous schema.
`test_extract_above_projection_that_redefines_column_name` covers the
recovery projection that must keep a rename alive.
- Three plan snapshots of neighbouring tests lose a duplicate
pass-through column in the intermediate stage. The optimized plans of
those three tests do not change.

Commands and results:

```text
cargo test --profile ci -p datafusion-optimizer
  884, 26, 5 passed; 0 failed

cargo test --profile ci -p datafusion-sqllogictest --test sqllogictests
  520/520 files completed, 0 failures

cargo clippy --profile ci -p datafusion-optimizer --all-targets -- -D warnings
  clean
```

## Are there any user-facing changes?

Statements of this shape plan and run. There is no API change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <[email protected]>

Backport of apache#25478 (merged upstream as 233aeab).