Skip to content

Commit 446c2ea

Browse files
authored
Fix the percentile argument for ApproxPercentileCont must be Float64, not Decimal128(2, 1) (#4228)
* Percentile coerce to f64 from decimal * Added test * fix message
1 parent 123ca22 commit 446c2ea

File tree

3 files changed

+33
-8
lines changed

3 files changed

+33
-8
lines changed

datafusion/core/tests/sql/aggregates.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2373,3 +2373,24 @@ async fn array_agg_one() -> Result<()> {
23732373
assert_batches_eq!(expected, &actual);
23742374
Ok(())
23752375
}
2376+
2377+
#[tokio::test]
2378+
async fn test_approx_percentile_cont_decimal_support() -> Result<()> {
2379+
let ctx = SessionContext::new();
2380+
register_aggregate_csv(&ctx).await?;
2381+
let sql = "SELECT c1, approx_percentile_cont(c2, cast(0.85 as decimal(10,2))) apc FROM aggregate_test_100 GROUP BY 1 ORDER BY 1";
2382+
let actual = execute_to_batches(&ctx, sql).await;
2383+
let expected = vec![
2384+
"+----+-----+",
2385+
"| c1 | apc |",
2386+
"+----+-----+",
2387+
"| a | 4 |",
2388+
"| b | 5 |",
2389+
"| c | 4 |",
2390+
"| d | 4 |",
2391+
"| e | 4 |",
2392+
"+----+-----+",
2393+
];
2394+
assert_batches_eq!(expected, &actual);
2395+
Ok(())
2396+
}

datafusion/expr/src/type_coercion/aggregates.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ use std::ops::Deref;
2323

2424
use crate::{AggregateFunction, Signature, TypeSignature};
2525

26+
use super::functions::can_coerce_from;
27+
2628
pub static STRINGS: &[DataType] = &[DataType::Utf8, DataType::LargeUtf8];
2729

2830
pub static NUMERICS: &[DataType] = &[
@@ -166,19 +168,22 @@ pub fn coerce_types(
166168
agg_fun, input_types[0]
167169
)));
168170
}
169-
if !matches!(input_types[1], DataType::Float64) {
170-
return Err(DataFusionError::Plan(format!(
171-
"The percentile argument for {:?} must be Float64, not {:?}.",
172-
agg_fun, input_types[1]
173-
)));
174-
}
175171
if input_types.len() == 3 && !is_integer_arg_type(&input_types[2]) {
176172
return Err(DataFusionError::Plan(format!(
177173
"The percentile sample points count for {:?} must be integer, not {:?}.",
178174
agg_fun, input_types[2]
179175
)));
180176
}
181-
Ok(input_types.to_vec())
177+
let mut result = input_types.to_vec();
178+
if can_coerce_from(&DataType::Float64, &input_types[1]) {
179+
result[1] = DataType::Float64;
180+
} else {
181+
return Err(DataFusionError::Plan(format!(
182+
"Could not coerce the percent argument for {:?} to Float64. Was {:?}.",
183+
agg_fun, input_types[1]
184+
)));
185+
}
186+
Ok(result)
182187
}
183188
AggregateFunction::ApproxPercentileContWithWeight => {
184189
if !is_approx_percentile_cont_supported_arg_type(&input_types[0]) {

datafusion/physical-expr/src/aggregate/tdigest.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,6 @@ impl TDigest {
231231
}
232232

233233
pub(crate) fn merge_sorted_f64(&self, sorted_values: &[f64]) -> TDigest {
234-
dbg!(&sorted_values);
235234
#[cfg(debug_assertions)]
236235
debug_assert!(is_sorted(sorted_values), "unsorted input to TDigest");
237236

0 commit comments

Comments
 (0)