You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ozenyunus35 (via GitHub)" <gi...@apache.org> on 2026/10/14 15:09:32 UTC
[PR] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
ozenyunus35 opened a new pull request, #26312:
URL: https://github.com/apache/datafusion/pull/26312
## Which issue does this PR close?
- Closes #26308.
## Rationale for this change
The `Wildcard` and `QualifiedWildcard` arms of `project_with_validation` repeated the same REPLACE handling and the same normalize/columnize step, and the `Expression` arm repeated that step a third time.
## What changes are included in this PR?
- A `push_validated` closure in the loop does the normalize/columnize (or pass-through) push, and all three arms use it.
- A small `apply_replace` helper applies a wildcard's REPLACE clause if there is one.
Each wildcard arm now only computes its expansion. No behaviour change.
## What is the testing strategy for this PR?
Covered by existing tests: the builder unit tests in `datafusion-expr` and the wildcard / `REPLACE` cases in `select.slt`, `unnest.slt`, `functions.slt` and `aggregate.slt`, which all pass locally.
## Are there any user-facing changes?
No.
I used an AI assistant to help draft parts of the code; I reviewed the changes and ran the tests myself.
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6453379238
A follow on sounds good. Thank you @ozenyunus35 and @andygrove
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6398810979
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-expr v55.1.0 (current)
Built [ 31.384s] (current)
Parsing datafusion-expr v55.1.0 (current)
Parsed [ 0.086s] (current)
Building datafusion-expr v55.1.0 (baseline)
Built [ 25.359s] (baseline)
Parsing datafusion-expr v55.1.0 (baseline)
Parsed [ 0.090s] (baseline)
Checking datafusion-expr v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 1.726s] 223 checks: 215 pass, 8 fail, 0 warn, 31 skip
--- failure enum_missing: pub enum removed or renamed ---
Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/enum_missing.ron
Failed in:
enum datafusion_expr::logical_plan::materialized_cte::MaterializedCtePlan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:70
enum datafusion_expr::materialized_cte::MaterializedCtePlan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:70
enum datafusion_expr::logical_plan::MaterializedCtePlan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:70
enum datafusion_expr::MaterializedCtePlan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:70
--- failure enum_variant_missing: pub enum variant removed or renamed ---
Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/enum_variant_missing.ron
Failed in:
variant LogicalPlan::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/plan.rs:370
variant LogicalPlan::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/plan.rs:370
--- failure function_missing: pub fn removed or renamed ---
Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/function_missing.ron
Failed in:
function datafusion_expr::utils::passthrough_field_index, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/utils.rs:1055
function datafusion_expr::utils::collect_subquery_join_exprs, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/utils.rs:1613
--- failure inherent_method_missing: pub method removed or renamed ---
Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/inherent_method_missing.ron
Failed in:
ScalarUDF::supports_range_key_locality_analysis, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/udf.rs:368
--- failure module_missing: pub module removed or renamed ---
Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/module_missing.ron
Failed in:
mod datafusion_expr::logical_plan::materialized_cte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:18
mod datafusion_expr::materialized_cte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:18
--- failure struct_missing: pub struct removed or renamed ---
Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/struct_missing.ron
Failed in:
struct datafusion_expr::logical_plan::materialized_cte::MaterializedCteId, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:47
struct datafusion_expr::materialized_cte::MaterializedCteId, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:47
struct datafusion_expr::logical_plan::MaterializedCteId, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:47
struct datafusion_expr::MaterializedCteId, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:47
struct datafusion_expr::logical_plan::materialized_cte::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:88
struct datafusion_expr::materialized_cte::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:88
struct datafusion_expr::logical_plan::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:88
struct datafusion_expr::MaterializedCte, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:88
struct datafusion_expr::logical_plan::materialized_cte::MaterializedCteScan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:196
struct datafusion_expr::materialized_cte::MaterializedCteScan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:196
struct datafusion_expr::logical_plan::MaterializedCteScan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:196
struct datafusion_expr::MaterializedCteScan, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/logical_plan/materialized_cte.rs:196
--- failure trait_method_missing: pub trait method removed or renamed ---
Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-item-signature
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/trait_method_missing.ron
Failed in:
method plan_at_time_zone of trait ExprPlanner, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/planner.rs:232
method evaluate_ranges of trait PartitionEvaluator, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/partition_evaluator.rs:215
method supports_range_key_locality_analysis of trait ScalarUDFImpl, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/e2c0726845882c3d3de9ca1d99b27562053b2a70/datafusion/expr/src/udf.rs:1014
--- failure trait_method_parameter_count_changed: pub trait method parameter count changed ---
Description:
A trait method now takes a different number of parameters.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-item-signature
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/trait_method_parameter_count_changed.ron
Failed in:
ExprPlanner::plan_scalar now takes 1 instead of 2 parameters, in file /home/runner/work/datafusion/datafusion/datafusion/expr/src/planner.rs:269
Summary semver requires new major version: 8 major and 0 minor checks failed
Finished [ 60.360s] datafusion-expr
```
</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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #26312:
URL: https://github.com/apache/datafusion/pull/26312
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "ozenyunus35 (via GitHub)" <gi...@apache.org>.
ozenyunus35 commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6374842463
@alamb Friendly ping when you have a moment: this small refactor is mergeable and ready for review. Happy to change anything.
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6376982396
@andygrove can you please help review this PR as you filed the issue request?
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "ozenyunus35 (via GitHub)" <gi...@apache.org>.
ozenyunus35 commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6400493827
@andygrove Thanks for reading the whole diff so carefully! I'll keep the two nits in mind and leave this PR as is, unless you'd prefer them here.
--
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] refactor: share the REPLACE and validation steps of the wildcard arms in project_with_validation [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #26312:
URL: https://github.com/apache/datafusion/pull/26312#issuecomment-6399025509
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/26312?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `77.27273%` with `5 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.79%. Comparing base ([`0d79252`](https://app.codecov.io/gh/apache/datafusion/commit/0d79252f61bd8777990b2749c216ffd07079c30d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0fb8f9b`](https://app.codecov.io/gh/apache/datafusion/commit/0fb8f9bdd5aca2e17addfb29b132e090894979f1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 211 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/26312?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/expr/src/logical\_plan/builder.rs](https://app.codecov.io/gh/apache/datafusion/pull/26312?src=pr&el=tree&filepath=datafusion%2Fexpr%2Fsrc%2Flogical_plan%2Fbuilder.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9sb2dpY2FsX3BsYW4vYnVpbGRlci5ycw==) | 77.27% | [0 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/26312?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #26312 +/- ##
==========================================
- Coverage 82.80% 82.79% -0.01%
==========================================
Files 1149 1149
Lines 454146 454146
Branches 454146 454146
==========================================
- Hits 376035 376030 -5
- Misses 55260 55261 +1
- Partials 22851 22855 +4
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/26312?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