adriangb opened a new pull request, #25452: URL: https://github.com/apache/datafusion/pull/25452
## Which issue does this PR close? - Part of https://github.com/apache/datafusion/issues/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 (#23655), and two sites that reference a plain column only. The pass-through case is the one that https://github.com/apache/datafusion/issues/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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
