You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "bert-beyondloops (via GitHub)" <gi...@apache.org> on 2026/09/15 11:03:31 UTC
[PR] fix: don't push get_field-like leaf expressions through a CSE common_expr [datafusion]
bert-beyondloops opened a new pull request, #25330:
URL: https://github.com/apache/datafusion/pull/25330
- Closes #25329
## Rationale for this change
`CommonSubexprEliminate` extracts a repeated sub-expression into its own `__common_expr_N` projection column so it's computed once instead of once per occurrence.
Separately, `extract_leaf_expressions` (the leaf-pushdown optimizer) tries to push cheap, `MoveTowardsLeafNodes`-classified expressions down closer to the base table scan.
The bug: once CSE rewrites a repeated expression to a bare `Column` reference to `__common_expr_N`, that reference is indistinguishable from a genuine cheap base-table column at the `ExpressionPlacement` level. If it's wrapped in something like `get_field(__common_expr_N, 'name')`, the leaf-pushdown pass can classify the whole thing as `MoveTowardsLeafNodes` and push it down — which requires re-deriving `__common_expr_N`'s original (potentially expensive) defining expression at the new, lower location. That silently duplicates the exact computation CSE had just deduplicated, defeating the purpose of CSE for any expression that also gets leaf-pushed.
## What changes are included in this PR?
- Add `references_cse_common_expr` in `common_subexpr_eliminate.rs`, which checks whether an expression contains a column reference to a CSE-synthesized `__common_expr_N` alias.
- In `extract_leaf_expressions.rs`, short-circuit `routing_extract` for `MoveTowardsLeafNodes` expressions that reference a CSE common-expr column:
leave them exactly where CSE placed them instead of extracting/pushing them down.
## What is the testing strategy for this PR?
Added a new `sqllogictest` case in `datafusion/sqllogictest/test_files/cse.slt`
## Are there any user-facing changes?
No public API changes. This is a bug fix affecting query plans: some queries combining CSE-eligible sub-expressions with leaf-pushdown-eligible wrapping expressions (e.g. `get_field`) will now correctly evaluate the common sub-expression once instead of duplicating it, which may change generated `EXPLAIN` output but not query results (results were already correct; only redundant computation is removed).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: don't push get_field-like leaf expressions through a CSE common_expr [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25330:
URL: https://github.com/apache/datafusion/pull/25330#issuecomment-5679673410
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25330?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 81.92%. Comparing base ([`133111f`](https://app.codecov.io/gh/apache/datafusion/commit/133111f43a579260df4b0d4e76e2537a46cadda9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2c2a6de`](https://app.codecov.io/gh/apache/datafusion/commit/2c2a6de7b9ada4060d2afa0582f97433259862c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25330 +/- ##
========================================
Coverage 81.92% 81.92%
========================================
Files 1134 1134
Lines 426000 426314 +314
Branches 426000 426314 +314
========================================
+ Hits 349000 349268 +268
- Misses 56304 56331 +27
- Partials 20696 20715 +19
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25330?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: don't push get_field-like leaf expressions through a CSE common_expr [datafusion]
Posted by "bert-beyondloops (via GitHub)" <gi...@apache.org>.
bert-beyondloops closed pull request #25330: fix: don't push get_field-like leaf expressions through a CSE common_expr
URL: https://github.com/apache/datafusion/pull/25330
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: don't push get_field-like leaf expressions through a CSE common_expr [datafusion]
Posted by "bert-beyondloops (via GitHub)" <gi...@apache.org>.
bert-beyondloops commented on PR #25330:
URL: https://github.com/apache/datafusion/pull/25330#issuecomment-5695759345
Closed in favour of PR #23691
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org