adriangb commented on code in PR #21900:
URL: https://github.com/apache/datafusion/pull/21900#discussion_r3161571847
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -446,22 +511,27 @@ impl BatchPartitioner {
/// - `num_partitions`: Total number of output partitions.
/// - `timer`: Metric used to record time spent during repartitioning.
///
- /// # Notes
- /// This constructor cannot fail and performs no validation.
+ /// # Errors
+ /// Returns an error if `num_partitions` is zero.
pub fn new_hash_partitioner(
exprs: Vec<Arc<dyn PhysicalExpr>>,
num_partitions: usize,
timer: metrics::Time,
- ) -> Self {
- Self {
+ ) -> Result<Self> {
Review Comment:
Technically a breaking change but I think this is one of those things that
is more so public for the workspace than it is for end users. So may not be
worth calling out.
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -575,7 +645,8 @@ impl BatchPartitioner {
}
BatchPartitionerState::Hash {
exprs,
- num_partitions: partitions,
+ num_partitions: _,
Review Comment:
Dead code? Should we remove?
--
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]
