[ozenyunus35]

Which issue does this PR close?

• Closes Merge duplicated wildcard arms in LogicalPlanBuilder project_with_validation #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.

[ozenyunus35]

@alamb Friendly ping when you have a moment: this small refactor is mergeable and ready for review. Happy to change anything.

[alamb]

@andygrove can you please help review this PR as you filed the issue request?

[andygrove]

LGTM. I read the whole diff and the refactor preserves behaviour: the REPLACE step, the normalize-then-columnize step and the order of operations are unchanged. The borrows are sound, including opt.replace now being borrowed with .as_ref(). cargo check -p datafusion-expr passes on the PR head. I did not run the tests or clippy locally, so I'm relying on CI for those.

Two optional nits, neither blocking:

• The push_validated closure holds &mut borrows of projected_expr and normalizer, so nothing in the loop body can read them later. A small private helper function would avoid that.
• apply_replace is a thin wrapper around an Option match. replace_columns could take Option<&PlannedReplaceSelectItem> directly. The wildcard and qualified-wildcard arms still repeat the same expand, apply_replace, try_for_each chain, which could share a tail after the match.

[github-actions]

Thank you for opening this pull request!

Reviewer note: 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

     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

[codecov-commenter]

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.79%. Comparing base (0d79252) to head (0fb8f9b).
⚠️ Report is 211 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/expr/src/logical_plan/builder.rs	77.27%	0 Missing and 5 partials ⚠️

Additional details and impacted files

@@            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     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ozenyunus35]

@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.

[alamb]

A follow on sounds good. Thank you @ozenyunus35 and @andygrove