Skip to content

chore: remove enforce_batch_size_in_joins - #24832

Open
saadtajwar wants to merge 1 commit into
apache:mainfrom
saadtajwar:saadtajwar/remove-enforce-batch-size-in-joins
Open

chore: remove enforce_batch_size_in_joins#24832
saadtajwar wants to merge 1 commit into
apache:mainfrom
saadtajwar:saadtajwar/remove-enforce-batch-size-in-joins

Conversation

@saadtajwar

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

This config is only used for symmetric hash join, and outputting incrementally seems to be always better (the behavior when enforce_batch_size_in_joins set to true), so we can remove the config and default to this behavior

What changes are included in this PR?

  • Making SHJ always output incrementally
  • Deprecation notices for the config & related methods

What is the testing strategy for this PR?

Ran existing tests & updates tests that asserted the path with the config disabled

Are there any user-facing changes?

Yes - config and upgrade guide updated to reflect

@github-actions github-actions Bot added documentation Improvements or additions to documentation sqllogictest SQL Logic Tests (.slt) common Related to common crate execution Related to the execution crate physical-plan Changes to the physical-plan crate labels Aug 31, 2026
@github-actions

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-common v55.0.0 (current)
       Built [  34.581s] (current)
     Parsing datafusion-common v55.0.0 (current)
      Parsed [   0.066s] (current)
    Building datafusion-common v55.0.0 (baseline)
       Built [  33.321s] (baseline)
     Parsing datafusion-common v55.0.0 (baseline)
      Parsed [   0.066s] (baseline)
    Checking datafusion-common v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   1.103s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [  70.755s] datafusion-common
    Building datafusion-execution v55.0.0 (current)
       Built [  30.168s] (current)
     Parsing datafusion-execution v55.0.0 (current)
      Parsed [   0.029s] (current)
    Building datafusion-execution v55.0.0 (baseline)
       Built [  29.652s] (baseline)
     Parsing datafusion-execution v55.0.0 (baseline)
      Parsed [   0.030s] (baseline)
    Checking datafusion-execution v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.324s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip

--- failure type_method_marked_deprecated: type method #[deprecated] added ---

Description:
A type method is now #[deprecated]. Downstream crates will get a compiler warning when using this method.
        ref: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/type_method_marked_deprecated.ron

Failed in:
  method datafusion_execution::config::SessionConfig::with_enforce_batch_size_in_joins in /home/runner/work/datafusion/datafusion/datafusion/execution/src/config.rs:478
  method datafusion_execution::config::SessionConfig::enforce_batch_size_in_joins in /home/runner/work/datafusion/datafusion/datafusion/execution/src/config.rs:492

     Summary semver requires new minor version: 0 major and 1 minor checks failed
    Finished [  61.623s] datafusion-execution
    Building datafusion-physical-plan v55.0.0 (current)
       Built [  38.126s] (current)
     Parsing datafusion-physical-plan v55.0.0 (current)
      Parsed [   0.156s] (current)
    Building datafusion-physical-plan v55.0.0 (baseline)
       Built [  37.417s] (baseline)
     Parsing datafusion-physical-plan v55.0.0 (baseline)
      Parsed [   0.160s] (baseline)
    Checking datafusion-physical-plan v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   1.000s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [  78.269s] datafusion-physical-plan
    Building datafusion-sqllogictest v55.0.0 (current)
       Built [  98.533s] (current)
     Parsing datafusion-sqllogictest v55.0.0 (current)
      Parsed [   0.023s] (current)
    Building datafusion-sqllogictest v55.0.0 (baseline)
       Built [  98.382s] (baseline)
     Parsing datafusion-sqllogictest v55.0.0 (baseline)
      Parsed [   0.025s] (baseline)
    Checking datafusion-sqllogictest v55.0.0 -> v55.0.0 (no change; assume patch)
     Checked [   0.120s] 223 checks: 223 pass, 31 skip
     Summary no semver update required
    Finished [ 199.873s] datafusion-sqllogictest

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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.57%. Comparing base (a274959) to head (df721fd).

Files with missing lines Patch % Lines
...ion/physical-plan/src/joins/symmetric_hash_join.rs 83.33% 0 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24832      +/-   ##
==========================================
- Coverage   81.58%   81.57%   -0.01%     
==========================================
  Files        1123     1123              
  Lines      406610   406638      +28     
  Branches   406610   406638      +28     
==========================================
- Hits       331719   331714       -5     
- Misses      55453    55471      +18     
- Partials    19438    19453      +15     

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

@saadtajwar
saadtajwar marked this pull request as ready for review September 1, 2026 12:15
@saadtajwar

Copy link
Copy Markdown
Contributor Author

@2010YOUY01 - ready for review!

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 common Related to common crate documentation Improvements or additions to documentation execution Related to the execution crate physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove enforce_batch_size_in_joins in symmetric hash join, and deprecate the config

2 participants