stuhood commented on code in PR #24766:
URL: https://github.com/apache/datafusion/pull/24766#discussion_r4033306103
##########
datafusion/physical-expr/src/partitioning.rs:
##########
@@ -234,19 +248,75 @@ impl RangePartitioning {
Ok(Self::new(ordering, split_points))
}
+ /// Creates sample-backed range partitioning and validates the sample
shape,
+ /// ordering, and target partition count.
+ ///
+ /// `partition_count` must be at least one and no larger than
+ /// `samples.len() + 1`. When it is smaller than that maximum, the samples
+ /// are evenly down-sampled to derive the effective split points.
+ pub fn try_new_with_samples(
+ ordering: LexOrdering,
+ samples: Vec<SplitPoint>,
+ partition_count: usize,
+ ) -> Result<Self> {
Review Comment:
> @stuhood do you think there is a use case for having a contructor that
does not return a `Result`? I could see it if a use case vlaidated themselves
and is really trying to squeeze perf
Hm, maybe... but I can't think of any cases where you'd be creating
`RangePartitioning` instances in a tight loop.
--
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]
