saadtajwar commented on code in PR #26593:
URL: https://github.com/apache/datafusion/pull/26593#discussion_r4400773412
##########
datafusion/physical-plan/src/repartition/range.rs:
##########
@@ -37,6 +37,9 @@ pub(crate) struct RangeRouter {
split_points: Vec<SplitPoint>,
sort_options: Vec<SortOptions>,
inner: RangeRouterInner,
+ /// When outer bounds are included in `split_points`, only these partitions
+ /// are valid. Partitions before/after this range contain out-of-bounds
keys.
+ valid_partitions: Option<std::ops::Range<usize>>,
Review Comment:
meganit: would it be cleaner to `use std::ops::Range` and change the type
here to just `Range`?
##########
datafusion/common/src/partitioning.rs:
##########
@@ -89,10 +86,10 @@ pub fn validate_range_split_points(
split_points[0].values(),
split_points[1].values(),
sort_options,
- )? != Ordering::Less
+ )? == Ordering::Greater
Review Comment:
Hmm out of curiosity, can I ask if there's context around the addition of
equal split points? Is there a case for distributed execution in which it would
be helpful to have an empty partition that I may be missing?
> Another thing which might be useful is to be able to pad empty partitions.
Ex. use duplicate split points such as 10, 10, 10, 10 to indicate empty
partitions.
I see the above comment in #26509, but would definitely like to understand
use cases for the empty partitions :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
