From 26eb406f9ccfde8b701029da7fb9e9293f07f4e9 Mon Sep 17 00:00:00 2001 From: Alex Huang Date: Tue, 14 Mar 2023 14:26:58 +0100 Subject: [PATCH] refactor: add more error info when array is empty (#5560) * refactor: add more error info when array is empty * make code more concise * add tests and refine the error report --- .../core/tests/sqllogictests/test_files/aggregate.slt | 6 ++++++ .../physical-expr/src/aggregate/approx_percentile_cont.rs | 5 +++++ datafusion/physical-expr/src/aggregate/median.rs | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/datafusion/core/tests/sqllogictests/test_files/aggregate.slt b/datafusion/core/tests/sqllogictests/test_files/aggregate.slt index 694b062c7f78..6108f2212f53 100644 --- a/datafusion/core/tests/sqllogictests/test_files/aggregate.slt +++ b/datafusion/core/tests/sqllogictests/test_files/aggregate.slt @@ -1284,3 +1284,9 @@ NULL 2 statement ok drop table the_nulls; + +query error DataFusion error: Execution error: median requires at least one non-null value +select median(a) from (select 1 as a where 1=0); + +query error DataFusion error: Execution error: aggregate function needs at least one non-null element +select approx_median(a) from (select 1 as a where 1=0); diff --git a/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs b/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs index 670030b09e66..083671c6b55d 100644 --- a/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs +++ b/datafusion/physical-expr/src/aggregate/approx_percentile_cont.rs @@ -367,6 +367,11 @@ impl Accumulator for ApproxPercentileAccumulator { } fn evaluate(&self) -> Result { + if self.digest.count() == 0.0 { + return Err(DataFusionError::Execution( + "aggregate function needs at least one non-null element".to_string(), + )); + } let q = self.digest.estimate_quantile(self.percentile); // These acceptable return types MUST match the validation in diff --git a/datafusion/physical-expr/src/aggregate/median.rs b/datafusion/physical-expr/src/aggregate/median.rs index 9daabc01d8be..a29fd8fd7c0e 100644 --- a/datafusion/physical-expr/src/aggregate/median.rs +++ b/datafusion/physical-expr/src/aggregate/median.rs @@ -143,6 +143,12 @@ impl Accumulator for MedianAccumulator { } fn evaluate(&self) -> Result { + if !self.all_values.iter().any(|v| !v.is_null()) { + return Err(DataFusionError::Execution( + "median requires at least one non-null value".into(), + )); + } + // Create an array of all the non null values and find the // sorted indexes let array = ScalarValue::iter_to_array(