From 4ebbed8ece2303c0f24471fd44a2c31ca531e21b Mon Sep 17 00:00:00 2001 From: Matthew Van Schellebeeck Date: Wed, 16 Nov 2022 10:39:50 -0500 Subject: [PATCH 1/2] Make clippy happy --- datafusion/common/src/parsers.rs | 4 ++-- .../core/src/physical_plan/file_format/mod.rs | 1 + .../physical-expr/src/aggregate/count_distinct.rs | 3 +-- datafusion/physical-expr/src/aggregate/tdigest.rs | 15 +++------------ .../physical-expr/src/expressions/binary.rs | 14 +++++++------- datafusion/physical-expr/src/window/row_number.rs | 2 +- 6 files changed, 15 insertions(+), 24 deletions(-) diff --git a/datafusion/common/src/parsers.rs b/datafusion/common/src/parsers.rs index 94ad5d86be30..c591489f5198 100644 --- a/datafusion/common/src/parsers.rs +++ b/datafusion/common/src/parsers.rs @@ -111,7 +111,7 @@ pub fn parse_interval(leading_field: &str, value: &str) -> Result { let (diff_month, diff_days, diff_nanos) = calculate_from_part(interval_period_str.unwrap(), unit)?; - result_month += diff_month as i64; + result_month += diff_month; if result_month > (i32::MAX as i64) { return Err(DataFusionError::NotImplemented(format!( @@ -120,7 +120,7 @@ pub fn parse_interval(leading_field: &str, value: &str) -> Result { ))); } - result_days += diff_days as i64; + result_days += diff_days; if result_days > (i32::MAX as i64) { return Err(DataFusionError::NotImplemented(format!( diff --git a/datafusion/core/src/physical_plan/file_format/mod.rs b/datafusion/core/src/physical_plan/file_format/mod.rs index fbeab4cbf4c9..6d61f06a4a68 100644 --- a/datafusion/core/src/physical_plan/file_format/mod.rs +++ b/datafusion/core/src/physical_plan/file_format/mod.rs @@ -507,6 +507,7 @@ impl From for FileMeta { /// /// ParquetExec ///``` +#[allow(clippy::manual_filter)] pub(crate) fn get_output_ordering( base_config: &FileScanConfig, ) -> Option<&[PhysicalSortExpr]> { diff --git a/datafusion/physical-expr/src/aggregate/count_distinct.rs b/datafusion/physical-expr/src/aggregate/count_distinct.rs index 588b739097d5..943f7b632e07 100644 --- a/datafusion/physical-expr/src/aggregate/count_distinct.rs +++ b/datafusion/physical-expr/src/aggregate/count_distinct.rs @@ -182,8 +182,7 @@ impl Accumulator for DistinctCountAccumulator { .state_data_types .iter() .map(|state_data_type| { - let values = Box::new(Vec::new()); - ScalarValue::new_list(Some(*values), state_data_type.clone()) + ScalarValue::new_list(Some(Vec::new()), state_data_type.clone()) }) .collect::>(); diff --git a/datafusion/physical-expr/src/aggregate/tdigest.rs b/datafusion/physical-expr/src/aggregate/tdigest.rs index 6457f7025fc7..0a0c56b8b991 100644 --- a/datafusion/physical-expr/src/aggregate/tdigest.rs +++ b/datafusion/physical-expr/src/aggregate/tdigest.rs @@ -213,16 +213,6 @@ impl TDigest { } } - fn clamp(v: f64, lo: f64, hi: f64) -> f64 { - if v > hi { - hi - } else if v < lo { - lo - } else { - v - } - } - #[cfg(test)] pub(crate) fn merge_unsorted_f64(&self, unsorted_values: Vec) -> TDigest { let mut values = unsorted_values; @@ -523,7 +513,8 @@ impl TDigest { let value = self.centroids[pos].mean() + ((rank - t) / self.centroids[pos].weight() - 0.5) * delta; - Self::clamp(value, min, max) + + value.clamp(min, max) } /// This method decomposes the [`TDigest`] and its [`Centroid`] instances @@ -684,7 +675,7 @@ mod tests { let mut t = TDigest::new(10); for v in vals { - t = t.merge_unsorted_f64(vec![v as f64]); + t = t.merge_unsorted_f64(vec![v]); } assert_error_bounds!(t, quantile = 0.5, want = 1.0); diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs index a4904053086e..d69abf113541 100644 --- a/datafusion/physical-expr/src/expressions/binary.rs +++ b/datafusion/physical-expr/src/expressions/binary.rs @@ -2439,8 +2439,8 @@ mod tests { &[ Some(value), // 1.23 None, - Some((value - 1) as i128), // 1.22 - Some(value + 1), // 1.24 + Some(value - 1), // 1.22 + Some(value + 1), // 1.24 ], 10, 2, @@ -2561,10 +2561,10 @@ mod tests { let value: i128 = 123; let decimal_array = Arc::new(create_decimal_array( &[ - Some(value as i128), // 1.23 + Some(value), // 1.23 None, - Some(value - 1), // 1.22 - Some((value + 1) as i128), // 1.24 + Some(value - 1), // 1.22 + Some(value + 1), // 1.24 ], 10, 2, @@ -2682,8 +2682,8 @@ mod tests { &[ Some(value), // 1.23 None, - Some((value - 1) as i128), // 1.22 - Some(value + 1), // 1.24 + Some(value - 1), // 1.22 + Some(value + 1), // 1.24 ], 10, 2, diff --git a/datafusion/physical-expr/src/window/row_number.rs b/datafusion/physical-expr/src/window/row_number.rs index d8ff215d476c..fb3bc770442f 100644 --- a/datafusion/physical-expr/src/window/row_number.rs +++ b/datafusion/physical-expr/src/window/row_number.rs @@ -65,7 +65,7 @@ impl BuiltInWindowFunctionExpr for RowNumber { &self, _batch: &RecordBatch, ) -> Result> { - Ok(Box::new(NumRowsEvaluator::default())) + Ok(Box::::default()) } } From 7a0df51bbde621a299784a73053d9f957e954799 Mon Sep 17 00:00:00 2001 From: Matthew Van Schellebeeck Date: Wed, 16 Nov 2022 13:49:39 -0500 Subject: [PATCH 2/2] std clamp panics if lo < hi: revert --- datafusion/core/src/physical_plan/file_format/mod.rs | 1 - datafusion/physical-expr/src/aggregate/tdigest.rs | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/datafusion/core/src/physical_plan/file_format/mod.rs b/datafusion/core/src/physical_plan/file_format/mod.rs index 6d61f06a4a68..fbeab4cbf4c9 100644 --- a/datafusion/core/src/physical_plan/file_format/mod.rs +++ b/datafusion/core/src/physical_plan/file_format/mod.rs @@ -507,7 +507,6 @@ impl From for FileMeta { /// /// ParquetExec ///``` -#[allow(clippy::manual_filter)] pub(crate) fn get_output_ordering( base_config: &FileScanConfig, ) -> Option<&[PhysicalSortExpr]> { diff --git a/datafusion/physical-expr/src/aggregate/tdigest.rs b/datafusion/physical-expr/src/aggregate/tdigest.rs index 0a0c56b8b991..dc9b5bec654b 100644 --- a/datafusion/physical-expr/src/aggregate/tdigest.rs +++ b/datafusion/physical-expr/src/aggregate/tdigest.rs @@ -213,6 +213,16 @@ impl TDigest { } } + fn clamp(v: f64, lo: f64, hi: f64) -> f64 { + if v > hi { + hi + } else if v < lo { + lo + } else { + v + } + } + #[cfg(test)] pub(crate) fn merge_unsorted_f64(&self, unsorted_values: Vec) -> TDigest { let mut values = unsorted_values; @@ -514,7 +524,7 @@ impl TDigest { let value = self.centroids[pos].mean() + ((rank - t) / self.centroids[pos].weight() - 0.5) * delta; - value.clamp(min, max) + Self::clamp(value, min, max) } /// This method decomposes the [`TDigest`] and its [`Centroid`] instances