Range partition on primitives. - #24598
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24598 +/- ##
==========================================
+ Coverage 81.58% 81.61% +0.03%
==========================================
Files 1123 1125 +2
Lines 406610 412785 +6175
Branches 406610 412785 +6175
==========================================
+ Hits 331719 336902 +5183
- Misses 55453 56020 +567
- Partials 19438 19863 +425 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5579f93 to
a6a8368
Compare
|
Nice results! |
gene-bordegaray
left a comment
gene-bordegaray
left a comment
There was a problem hiding this comment.
some optimization things and linear scan path that might be interesting 👍
|
here are my samplys:
You will see the cachine |
6f4a2df to
ccaab13
Compare
gene-bordegaray
left a comment
gene-bordegaray
left a comment
There was a problem hiding this comment.
i think there is some strctural things that could be improved with responsiblities. I prposed a rough idea. Let me know what you think @stuhood
d87522a to
06849b5
Compare
gene-bordegaray
left a comment
gene-bordegaray
left a comment
There was a problem hiding this comment.
starting to get there. I think this is last major round of comments
gene-bordegaray
left a comment
gene-bordegaray
left a comment
There was a problem hiding this comment.
oops did not mean to approve
06849b5 to
f387d6d
Compare
f387d6d to
612381c
Compare
|
Split out the benchmarks into the bottom commit, and applied review feedback in the top commit. The initial benchmark results were a bit optimistic (they never regressed during review feedback: they were just a bit off when they were initially posted). Now updated. Thanks for the feedback! |
|
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 |
Hm... is this not allowed for commits landing on |
jayzhan211
left a comment
jayzhan211
left a comment
There was a problem hiding this comment.
Thanks @stuhood! I've left two suggested changes below.
|
Sorry for the delay! Applied. |
|
Is this PR ready to merge? 🚀 |
|
I believe so, yes: the last change I pushed was to avoid changing method signatures. Note that there are two deprecations added that the bot is complaining about, but I'd be surprised if that violated the deprecation policy...? |
6 participants
Which issue does this PR close?
Rationale for this change
In query plans that use range re-partitioning (such as matching the range partitioning of underlying base data, or evaluating dynamic range filters in hash joins), range re-partitioning can represent a significant fraction of total query execution time.
As detailed in the issue, the existing implementation in
RepartitionExec/BatchPartitionerandRangeExprassigns rows to partitions on a row-by-row basis usingextract_row_at_idx_to_bufand dynamicScalarValuecomparisons. This allocates per row and has dynamic dispatch overhead, making range re-partitioning 15x–30x slower than hash re-partitioning on standard integer keys.This PR adds a zero-allocation, vectorizable implementation, achieving 10x–16x speedup on primitive numeric keys and 2x–3x speedup on string and composite keys.
What changes are included in this PR?
RangeRouterwith specialized routing paths:RowConverterstoring flatRowsbuffers for strings, decimals, dictionary arrays, and composite keys.RepartitionExecto construct and share anArc<RangeRouter>across input partitions, avoiding redundant split-point encoding.BatchPartitionerandRangeExprto delegate partition routing toRangeRouter.BatchPartitioner::try_new_range_partitionerand deprecatesBatchPartitioner::new_range_partitioner.Benchmark Results (vs
mainbaseline)main)range_repartition_i64_uniformrange_repartition_i64_sequentialrange_repartition_utf8_uniformrange_repartition_composite_i64Are these changes tested?
Added unit and integration tests.
Are there any user-facing changes?
BatchPartitioner::try_new_range_partitioner(&RangePartitioning, &Schema, Time) -> Result<Self>, which takes the input&Schemato infer column data types and coerce split points.BatchPartitioner::new_range_partitionerin favor oftry_new_range_partitioner.RangeExpr:RangeExpr::try_new_with_schema(Vec<PhysicalExprRef>, &RangePartitioning, &Schema) -> Result<Self>, ensuring routing matches key column data types (precision, scale, timezone).RangeExpr::try_new(Vec<PhysicalExprRef>, &RangePartitioning)in favor oftry_new_with_schema.