adriangb opened a new issue, #25451:
URL: https://github.com/apache/datafusion/issues/25451
## Is your feature request related to a problem or challenge?
`datafusion/optimizer/src/extract_leaf_expressions.rs` received
approximately 10 bug fixes since February 2026. Most of them add a guard that
stops a rewrite in one specific case. Each fix went in 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
guard can therefore become wrong in one direction, or stop firing, and the
suite stays green.
To measure this, I ran `cargo-mutants` over the two files that own filter
pushdown and leaf expression pushdown. `cargo-mutants` changes one expression
at a time, then runs the tests. A mutant that survives is a change to the code
that no test detects.
25 of 220 mutants survive. Some of them sit in the guards that the recent
bug fixes added.
## Describe the solution you'd like
Add tests that pin the decision each guard makes, and not only the plan that
comes out of the rule.
### Scope and command
Tool: `cargo-mutants` 27.1.0. Base commit: `3a647e49dd`.
```bash
cargo mutants \
-p datafusion-optimizer \
-f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
-f 'datafusion/optimizer/src/push_down_filter.rs' \
--profile ci \
--timeout 600 \
--jobs 2
```
This ran the complete set of mutants for the two files. No sharding was
used, so the fraction is 220 of 220. The run took 18 minutes.
The baseline runs the tests of the `datafusion-optimizer` package only. That
is 882 library tests, 26 integration tests and 5 documentation tests.
### Totals
| Outcome | Count |
|---|---|
| Caught | 139 |
| Missed | 25 |
| Unviable | 56 |
| Timeout | 0 |
| Total | 220 |
An unviable mutant does not compile. It is not a test gap.
### Survivors in `extract_leaf_expressions.rs` (15)
| Function | Mutation | What no test asserts |
|---|---|---|
| `passthrough_column` | guard `col.name == alias.name` to `true` | A
renaming alias (`test.a AS b`) is not a pass-through column. If it counts as
one, the recovery projection that restores the name is dropped. |
| `merge_would_duplicate_kept_expr` | `&&` to `\|\|` | Plain columns are not
counted as reference sites. Only a `KeepInPlace` column pins a merge. |
| `merge_would_duplicate_kept_expr` | `>` to `>=` | One reference to a
`KeepInPlace` column still merges. The guard can become maximally conservative
and block every merge. |
| `is_pure_extraction_projection` | body to `true` | A plan that is not a
projection stops the second push. |
| `is_pure_extraction_projection` | guard
`alias.name.starts_with(EXTRACTED_EXPR_PREFIX)` to `true` | An alias with a
different prefix, for example `__common_expr_1`, stops the second push. |
| `push_extraction_pairs` | guard `proj_exprs_captured == proj.expr.len()`
to `true` | The merge into a child projection is skipped when an outer
projection expression was not captured. The code comment says such expressions
are lost in the merge. |
| `try_push_into_inputs` | `==` to `!=` in the alias check | The rule bails
out when a merge deduplicates a requested alias away, so the alias is missing
from the merged output. |
| `split_and_push_projection` | delete `!` on `has_existing_extracted` | A
projection that holds only pre-existing `__datafusion_extracted` aliases, and
no new extractable expression, is still pushed further toward the scan. |
| `routing_extract` | delete match arm `ExpressionPlacement::Column` | A
plain column inside a mixed expression is added to `columns_needed`. Without it
the extraction projection can lose a pass-through column. |
| `has_extractable_expr` (2 mutants) | body to `true`, and `==` to `!=` |
Nothing. Both mutants make the pre-check always say yes. The full pipeline then
finds nothing and returns `None`, so the plan does not change. This guard
controls speed only. |
| `ExtractLeafExpressions::name`, `PushDownLeafProjections::name` (4
mutants) | to `""` and to `"xyzzy"` | The rule name. The name goes to optimizer
logs and to `skip_failed_rules` messages. Low value. |
### Survivors in `push_down_filter.rs` (10)
| Function | Mutation | What no test asserts |
|---|---|---|
| `can_evaluate_as_join_condition` | body to `Ok(true)` | A predicate that
holds a subquery (`EXISTS`, `IN`, a set comparison) is rejected as a join
condition. |
| `extract_or_clause` | delete match arm `Operator::Or` | The rule walks the
`OR` structure of a join predicate recursively. Without the arm, an `OR`
predicate no longer produces a pushable clause for one join side. |
| `PushDownFilter::rewrite` (line 835) | `\|\|` to `&&` on `count_changed
\|\| reorder_changed` | A predicate reorder alone, with no change to the number
of conjuncts, rebuilds the filter. |
| `PushDownFilter::rewrite` (line 957) | `\|\|` to `&&` on
`contains_list_columns \|\| contains_struct_columns` | A filter over `Unnest`
that touches the list columns only, or the struct columns only. The two
branches are not told apart. |
| `PushDownFilter::rewrite` (line 1089) | delete match arm
`Expr::Alias(alias)` | A filter is pushed through the partition keys of an
**aliased** window function. No test has an aliased window function above a
filter. |
| `PushDownFilter::name` (2 mutants) | to `""` and to `"xyzzy"` | The rule
name. Low value. |
| `make_filter` | body to `Ok(Default::default())` | Nothing. The function
is deprecated since 55.0.0 and has no caller in the repository. |
| `PushDownFilter::supports_rewrite` | body to `false` | Nothing.
`OptimizerRule::supports_rewrite` is deprecated since 47.0.0 with the note
"This method is no longer used", and it has no call site. The override can be
removed. |
| `with_debug_timing` | delete `!` on `log_enabled!(Level::Debug)` |
Nothing. The wrapper only adds a debug log line. Both branches call `f()` and
return its result. |
### Notes
Nine of the 25 survivors are noise: the two `has_extractable_expr` mutants,
the six rule name mutants, and the `with_debug_timing` mutant. Two more point
at dead code: `make_filter` and `supports_rewrite`. The other 14 are real gaps.
These guards are fully covered. Every mutant in them is caught:
- `would_duplicate_volatile` and `volatile_output_columns`.
- The Aggregate branch volatile check from
https://github.com/apache/datafusion/pull/25416.
- The volatile and `MoveTowardsLeafNodes` partition in `rewrite_projection`.
- `find_owning_input`. It already has a direct unit test.
- The `needs_recovery` check in `split_and_push_projection`.
These guards are not covered:
- `merge_would_duplicate_kept_expr`, `passthrough_column`,
`is_pure_extraction_projection`, `push_extraction_pairs` and
`try_push_into_inputs`.
The pattern is clear. A guard that has a direct unit test survives no
mutants. A guard that has only plan snapshot tests survives one or two.
One limit of the method is important. Mutation testing flips an operator
that the code already has. It cannot show that a guard compares the wrong
thing. The `needs_recovery` check is an example. Its mutant is caught, but the
check compares unqualified field names only, and it ignores qualifiers and data
types. That is the root cause of
https://github.com/apache/datafusion/issues/25414. A green mutation score for a
guard is not proof that the guard is correct.
## Describe alternatives you've considered
Adding more plan snapshot tests. This does not help. A snapshot test asserts
the output of the whole pipeline. It cannot show which guard produced that
output, and it passes when a guard becomes more conservative than it needs to
be.
Running `cargo-mutants` in CI. The run takes 18 minutes for two files on a
warm cache. A full workspace run is not practical today. A targeted run on
changed files, with `--in-diff`, is possible later.
## Additional context
### How to re-run
```bash
cargo install cargo-mutants
git checkout 3a647e49dd
# List the mutants first. This is fast.
cargo mutants --list \
-p datafusion-optimizer \
-f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
-f 'datafusion/optimizer/src/push_down_filter.rs'
# Run them. Use --output to keep the report.
cargo mutants \
-p datafusion-optimizer \
-f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
-f 'datafusion/optimizer/src/push_down_filter.rs' \
--profile ci --timeout 600 --jobs 2 \
--output /tmp/mutants
```
Read `/tmp/mutants/mutants.out/missed.txt` for the survivors. `caught.txt`,
`unviable.txt` and `timeout.txt` hold the other outcomes.
To confirm that a new test kills one mutant, filter by function name:
```bash
cargo mutants -p datafusion-optimizer \
-f 'datafusion/optimizer/src/extract_leaf_expressions.rs' \
-F 'passthrough_column' --profile ci --timeout 600 --jobs 2
```
`sccache` makes a large difference. Each mutant needs one incremental build
of the `datafusion-optimizer` crate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
