refactor(asof_join): enhance match value handling and comparator caching - #24519
refactor(asof_join): enhance match value handling and comparator caching#24519jayzhan211 wants to merge 1 commit into
Conversation
|
@2010YOUY01 @Xuanwo Could you review this when you have a moment? Thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24519 +/- ##
==========================================
- Coverage 81.29% 81.29% -0.01%
==========================================
Files 1116 1116
Lines 395460 395571 +111
Branches 395460 395571 +111
==========================================
+ Hits 321486 321575 +89
- Misses 55167 55177 +10
- Partials 18807 18819 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you! This PR does 1) perf 2) fix float +0.0 and -0.0 normalization, right? I have some thoughts on the zero normalization part: Is it possible to add an optimizer rule, to rewrite the expression like Other implementations seem can also benefit: #22835 |
I have concerns for moving this out as optimizer rule
|
|
I see, this makes sense to me. One extra challenge is how should we keep the +- 0.0 handling consistent in the entire project, and make this approach documented somewhere, otherwise different implementations will handle it differently #22835 Regarding the optimization part, I suggest we can wait until SQL and benchmark is done for #23738. Though this PR should be strictly better in performance, but we might be able to find some more aggressive optimization that need to structure the code differently, and it's easier to review when the benchmark is ready. Ideally we can proceed with a smaller fix-only PR right now, maybe #24375 from @Xuanwo |
Part of #23738
Rationale for this change
ASOF joins currently convert both match values into
ScalarValues for row-levelcomparisons. This adds repeated scalar construction and type dispatch to the
ordered scan hot path.
The match expressions are already evaluated into Arrow arrays for each input
batch, so the join can compare array positions directly using an Arrow
comparator constructed once per left/right batch pair.
What changes are included in this PR?
materialization.
preserve SQL semantics where
-0.0and+0.0compare equal.buffers, and forward-only scanning are important to the hot path.
Are these changes tested?
Yes.
-0.0and+0.0remain equal for ASOF match conditions.
comparison directions, shared build memory, and expression validation.
The following checks pass:
cargo test -p datafusion-physical-plan asof_join --libcargo clippy -p datafusion-physical-plan --lib -- -D warningscargo fmt --all -- --checkAre there any user-facing changes?
No API or query-semantic changes are intended.
This is an internal execution optimization. Floating-point signed-zero and
logical-NULL behavior are explicitly preserved.