Skip to content

perf(range): avoid eager ok_or in gen_range_timestamp - #25308

Open
niebayes wants to merge 2 commits into
apache:mainfrom
niebayes:perf/range_eager_ok_or
Open

niebayes wants to merge 2 commits into
apache:mainfrom
niebayes:perf/range_eager_ok_or

Conversation

@niebayes

@niebayes niebayes commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What I did

Replace per-row ok_or(exec_datafusion_err!(...)) with ok_or_else in gen_range_timestamp so the success path no longer allocates a DataFusionError for every generated row (speed up ~5x on 100k rows). Add a criterion bench for the timestamp range path.

# Before, with eager ok_or
range_timestamp_100000  time:   [60.762 ms 60.815 ms 60.868 ms]

# After, with lazy ok_or_else
range_timestamp_100000  time:   [12.390 ms 12.401 ms 12.413 ms]
                        change: [−79.634% −79.608% −79.582%] (p = 0.00 < 0.05)
                        Performance has improved.

Which issue does this PR close?

Rationale for this change

Avoid eager ok_or on the hot path.

What changes are included in this PR?

Replaces eager ok_or with lazy ok_or_else.

What is the testing strategy for this PR?

Added a criterion bench.

Are there any user-facing changes?

No

…mp generation

Replace per-row ok_or(exec_datafusion_err!(...)) with ok_or_else in
gen_range_timestamp so the success path no longer allocates a
DataFusionError for every generated row (~5x on 100k rows). Add a
criterion bench for the timestamp range path.
@github-actions github-actions Bot added the functions Changes to functions implementation label Sep 15, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.91%. Comparing base (add66e4) to head (be6bd0b).

Files with missing lines Patch % Lines
datafusion/functions-nested/src/range.rs 0.00% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25308      +/-   ##
==========================================
- Coverage   81.91%   81.91%   -0.01%     
==========================================
  Files        1134     1134              
  Lines      425655   425658       +3     
  Branches   425655   425658       +3     
==========================================
- Hits       348688   348660      -28     
- Misses      56288    56313      +25     
- Partials    20679    20685       +6     

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

@AdamGS AdamGS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

range function uses eager ok_or on the hot path

3 participants