Skip to content

refactor: move lambda variable scope into Physical Planning Context - #23989

Open
sweb wants to merge 1 commit into
apache:mainfrom
sweb:refactor/move-lambda-eval
Open

refactor: move lambda variable scope into Physical Planning Context#23989
sweb wants to merge 1 commit into
apache:mainfrom
sweb:refactor/move-lambda-eval

Conversation

@sweb

@sweb sweb commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

This is a clean up following #23649

What changes are included in this PR?

  • Moves the lambda-variable scope state out of ExecutionProps and into PhysicalPlanningContext, following the same pattern as the scalar-subquery state in refactor: pass PhysicalPlanningContext explicitly through planner traits #23649.

  • Puts PhysicalPlanningContext::indexes behind an Arc so the new per-lambda-body clone doesn't deep-copy the subquery index map. PhysicalPlanningContext::new's signature is unchanged.

Are these changes tested?

  • New lambda_variables_shadow_outer_scope unit test in physical_planning_context.rs covers with_qualified_lambda_variables / lambda_variable_qualifier directly, including an inner lambda shadowing an outer parameter name.

Are there any user-facing changes?

this is a breaking change for lib users.

@github-actions github-actions Bot added documentation Improvements or additions to documentation logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates labels Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

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 v54.1.0 (current)
       Built [  32.272s] (current)
     Parsing datafusion-expr v54.1.0 (current)
      Parsed [   0.076s] (current)
    Building datafusion-expr v54.1.0 (baseline)
       Built [  29.243s] (baseline)
     Parsing datafusion-expr v54.1.0 (baseline)
      Parsed [   0.077s] (baseline)
    Checking datafusion-expr v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   1.279s] 223 checks: 221 pass, 2 fail, 0 warn, 30 skip

--- 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.49.0/src/lints/inherent_method_missing.ron

Failed in:
  ExecutionProps::with_qualified_lambda_variables, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/2890071362773cf46bd25e60741113edced69a4d/datafusion/expr/src/execution_props.rs:145

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field 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.49.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field lambda_variable_qualifier of struct ExecutionProps, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks/git-apache_main/2890071362773cf46bd25e60741113edced69a4d/datafusion/expr/src/execution_props.rs:66

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  64.098s] datafusion-expr
    Building datafusion-physical-expr v54.1.0 (current)
       Built [  30.622s] (current)
     Parsing datafusion-physical-expr v54.1.0 (current)
      Parsed [   0.052s] (current)
    Building datafusion-physical-expr v54.1.0 (baseline)
       Built [  30.965s] (baseline)
     Parsing datafusion-physical-expr v54.1.0 (baseline)
      Parsed [   0.051s] (baseline)
    Checking datafusion-physical-expr v54.1.0 -> v54.1.0 (no change; assume patch)
     Checked [   0.330s] 223 checks: 223 pass, 30 skip
     Summary no semver update required
    Finished [  63.213s] datafusion-physical-expr

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Jul 30, 2026
@codecov-commenter

codecov-commenter commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.75%. Comparing base (2f25454) to head (3b38dfa).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23989      +/-   ##
==========================================
- Coverage   80.75%   80.75%   -0.01%     
==========================================
  Files        1096     1096              
  Lines      373588   373604      +16     
  Branches   373588   373604      +16     
==========================================
+ Hits       301687   301695       +8     
- Misses      53898    53901       +3     
- Partials    18003    18008       +5     

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

@sweb sweb changed the title refactor: move lambda evaluation into Physical Planning Context refactor: move lambda variable scope into Physical Planning Context Jul 30, 2026
@sweb
sweb force-pushed the refactor/move-lambda-eval branch from 0bab196 to 3b38dfa Compare July 30, 2026 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto detected api change Auto detected API change documentation Improvements or additions to documentation logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move lambda evaluation into Physical Planning Context

2 participants