Skip to content

Commit 3845631

Browse files
committed
fix: avoid internal errors for OneOf signature mismatches
Signed-off-by: yaommen <myanstu@163.com>
1 parent 4010a55 commit 3845631

12 files changed

Lines changed: 538 additions & 54 deletions

File tree

datafusion/expr/src/type_coercion/functions.rs

Lines changed: 437 additions & 38 deletions
Large diffs are not rendered by default.

datafusion/expr/src/udaf.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ use std::vec;
2626

2727
use arrow::datatypes::{DataType, Field, FieldRef};
2828

29-
use datafusion_common::{Result, ScalarValue, Statistics, exec_err, not_impl_err};
29+
use datafusion_common::{
30+
DataFusionError, Result, ScalarValue, Statistics, exec_err, not_impl_err,
31+
};
3032
use datafusion_expr_common::dyn_eq::{DynEq, DynHash};
3133
use datafusion_expr_common::operator::Operator;
3234
use datafusion_physical_expr_common::physical_expr::PhysicalExpr;
@@ -517,6 +519,15 @@ pub trait AggregateUDFImpl: Debug + DynEq + DynHash + Send + Sync {
517519
/// types are accepted and the function's Volatility.
518520
fn signature(&self) -> &Signature;
519521

522+
/// Allows a function to provide a more actionable signature failure
523+
/// message than the generic `OneOf` fallback.
524+
fn diagnose_failed_signature(
525+
&self,
526+
_arg_types: &[DataType],
527+
) -> Option<DataFusionError> {
528+
None
529+
}
530+
520531
/// What [`DataType`] will be returned by this function, given the types of
521532
/// the arguments
522533
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType>;
@@ -1243,6 +1254,13 @@ impl AggregateUDFImpl for AliasedAggregateUDFImpl {
12431254
self.inner.signature()
12441255
}
12451256

1257+
fn diagnose_failed_signature(
1258+
&self,
1259+
arg_types: &[DataType],
1260+
) -> Option<DataFusionError> {
1261+
self.inner.diagnose_failed_signature(arg_types)
1262+
}
1263+
12461264
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
12471265
self.inner.return_type(arg_types)
12481266
}

datafusion/expr/src/udf.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use arrow::datatypes::{DataType, Field, FieldRef};
2828
#[cfg(debug_assertions)]
2929
use datafusion_common::assert_or_internal_err;
3030
use datafusion_common::config::ConfigOptions;
31-
use datafusion_common::{ExprSchema, Result, ScalarValue, not_impl_err};
31+
use datafusion_common::{DataFusionError, ExprSchema, Result, ScalarValue, not_impl_err};
3232
use datafusion_expr_common::dyn_eq::{DynEq, DynHash};
3333
use datafusion_expr_common::interval_arithmetic::Interval;
3434
use datafusion_expr_common::placement::ExpressionPlacement;
@@ -572,6 +572,15 @@ pub trait ScalarUDFImpl: Debug + DynEq + DynHash + Send + Sync {
572572
/// [`DataFusionError::Internal`]: datafusion_common::DataFusionError::Internal
573573
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType>;
574574

575+
/// Allows a function to provide a more actionable signature failure
576+
/// message than the generic `OneOf` fallback.
577+
fn diagnose_failed_signature(
578+
&self,
579+
_arg_types: &[DataType],
580+
) -> Option<DataFusionError> {
581+
None
582+
}
583+
575584
/// Create a new instance of this function with updated configuration.
576585
///
577586
/// This method is called when configuration options change at runtime
@@ -1033,6 +1042,13 @@ impl ScalarUDFImpl for AliasedScalarUDFImpl {
10331042
self.inner.signature()
10341043
}
10351044

1045+
fn diagnose_failed_signature(
1046+
&self,
1047+
arg_types: &[DataType],
1048+
) -> Option<DataFusionError> {
1049+
self.inner.diagnose_failed_signature(arg_types)
1050+
}
1051+
10361052
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
10371053
self.inner.return_type(arg_types)
10381054
}

datafusion/expr/src/udwf.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use crate::udf_eq::UdfEq;
3333
use crate::{
3434
Expr, PartitionEvaluator, Signature, function::WindowFunctionSimplification,
3535
};
36-
use datafusion_common::{Result, not_impl_err};
36+
use datafusion_common::{DataFusionError, Result, not_impl_err};
3737
use datafusion_doc::Documentation;
3838
use datafusion_expr_common::dyn_eq::{DynEq, DynHash};
3939
use datafusion_functions_window_common::expr::ExpressionArgs;
@@ -333,6 +333,15 @@ pub trait WindowUDFImpl: Debug + DynEq + DynHash + Send + Sync {
333333
/// types are accepted and the function's Volatility.
334334
fn signature(&self) -> &Signature;
335335

336+
/// Allows a function to provide a more actionable signature failure
337+
/// message than the generic `OneOf` fallback.
338+
fn diagnose_failed_signature(
339+
&self,
340+
_arg_types: &[DataType],
341+
) -> Option<DataFusionError> {
342+
None
343+
}
344+
336345
/// Returns the expressions that are passed to the [`PartitionEvaluator`].
337346
fn expressions(&self, expr_args: ExpressionArgs) -> Vec<Arc<dyn PhysicalExpr>> {
338347
expr_args.input_exprs().into()
@@ -514,6 +523,13 @@ impl WindowUDFImpl for AliasedWindowUDFImpl {
514523
self.inner.signature()
515524
}
516525

526+
fn diagnose_failed_signature(
527+
&self,
528+
arg_types: &[DataType],
529+
) -> Option<DataFusionError> {
530+
self.inner.diagnose_failed_signature(arg_types)
531+
}
532+
517533
fn expressions(&self, expr_args: ExpressionArgs) -> Vec<Arc<dyn PhysicalExpr>> {
518534
expr_args
519535
.input_exprs()

datafusion/functions-aggregate/src/average.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ use arrow::datatypes::{
3232
DurationSecondType, Field, FieldRef, Float64Type, TimeUnit, UInt64Type, i256,
3333
};
3434
use datafusion_common::types::{NativeType, logical_float64};
35-
use datafusion_common::{Result, ScalarValue, exec_err, not_impl_err};
35+
use datafusion_common::{
36+
DataFusionError, Result, ScalarValue, exec_err, not_impl_err, plan_datafusion_err,
37+
};
3638
use datafusion_expr::function::{AccumulatorArgs, StateFieldsArgs};
3739
use datafusion_expr::utils::format_state_name;
3840
use datafusion_expr::{
@@ -139,6 +141,18 @@ impl AggregateUDFImpl for Avg {
139141
&self.signature
140142
}
141143

144+
fn diagnose_failed_signature(
145+
&self,
146+
arg_types: &[DataType],
147+
) -> Option<DataFusionError> {
148+
match arg_types {
149+
[DataType::Boolean] => {
150+
Some(plan_datafusion_err!("Avg not supported for Boolean"))
151+
}
152+
_ => None,
153+
}
154+
}
155+
142156
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
143157
match &arg_types[0] {
144158
DataType::Decimal32(precision, scale) => {

datafusion/functions-aggregate/src/sum.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ use datafusion_common::types::{
3232
NativeType, logical_float64, logical_int8, logical_int16, logical_int32,
3333
logical_int64, logical_uint8, logical_uint16, logical_uint32, logical_uint64,
3434
};
35-
use datafusion_common::{HashMap, Result, ScalarValue, exec_err, not_impl_err};
35+
use datafusion_common::{
36+
DataFusionError, HashMap, Result, ScalarValue, exec_err, not_impl_err,
37+
plan_datafusion_err,
38+
};
3639
use datafusion_expr::expr::AggregateFunction;
3740
use datafusion_expr::expr_fn::cast;
3841
use datafusion_expr::function::{AccumulatorArgs, StateFieldsArgs};
@@ -213,6 +216,18 @@ impl AggregateUDFImpl for Sum {
213216
&self.signature
214217
}
215218

219+
fn diagnose_failed_signature(
220+
&self,
221+
arg_types: &[DataType],
222+
) -> Option<DataFusionError> {
223+
match arg_types {
224+
[DataType::Boolean] => {
225+
Some(plan_datafusion_err!("Sum not supported for Boolean"))
226+
}
227+
_ => None,
228+
}
229+
}
230+
216231
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
217232
match &arg_types[0] {
218233
DataType::Int64 => Ok(DataType::Int64),

datafusion/sqllogictest/test_files/array.slt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7896,17 +7896,17 @@ select generate_series(arrow_cast('2021-01-01T00:00:00', 'Timestamp(Nanosecond,
78967896
[2021-01-01T00:00:00-05:00, 2021-01-01T01:29:54.500-05:00, 2021-01-01T02:59:49-05:00, 2021-01-01T04:29:43.500-05:00, 2021-01-01T05:59:38-05:00]
78977897

78987898
## mixing types for timestamps is not supported
7899-
query error DataFusion error: Error during planning: Internal error: Function 'generate_series' failed to match any signature
7899+
query error DataFusion error: Error during planning: Function 'generate_series' failed to match any signature
79007900
select generate_series(arrow_cast('2021-01-01T00:00:00', 'Timestamp(Nanosecond, Some("-05:00"))'), DATE '2021-01-02', INTERVAL '1' HOUR);
79017901

79027902
## mixing types not allowed even if an argument is null
7903-
query error DataFusion error: Error during planning: Internal error: Function 'generate_series' failed to match any signature
7903+
query error DataFusion error: Error during planning: Function 'generate_series' failed to match any signature
79047904
select generate_series(TIMESTAMP '1992-09-01', DATE '1993-03-01', NULL);
79057905

7906-
query error DataFusion error: Error during planning: Internal error: Function 'generate_series' failed to match any signature
7906+
query error DataFusion error: Error during planning: Function 'generate_series' failed to match any signature
79077907
select generate_series(1, '2024-01-01', '2025-01-02');
79087908

7909-
query error DataFusion error: Error during planning: Internal error: Function 'generate_series' failed to match any signature
7909+
query error DataFusion error: Error during planning: Function 'generate_series' failed to match any signature
79107910
select generate_series('2024-01-01'::timestamp, '2025-01-02', interval '1 day');
79117911

79127912
## should return NULL

datafusion/sqllogictest/test_files/errors.slt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,19 @@ from aggregate_test_100
125125
order by c9
126126

127127
# WindowFunction wrong signature
128-
statement error DataFusion error: Error during planning: Internal error: Function 'nth_value' failed to match any signature
128+
statement error DataFusion error: Error during planning: Function 'nth_value' expects 0 to 2 arguments but received 3
129129
select
130130
c9,
131131
nth_value(c5, 2, 3) over (order by c9) as nv1
132132
from aggregate_test_100
133133
order by c9
134134

135+
query error DataFusion error: Error during planning: Sum not supported for Boolean
136+
select sum(bool_col) from (values (true), (false), (null)) as t(bool_col);
137+
138+
query error DataFusion error: Error during planning: Avg not supported for Boolean
139+
select avg(bool_col) from (values (true), (false), (null)) as t(bool_col);
140+
135141

136142
# nth_value with wrong name
137143
statement error DataFusion error: Error during planning: Invalid function 'nth_vlue'.\nDid you mean 'nth_value'?

datafusion/sqllogictest/test_files/functions.slt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,10 @@ SELECT substr('alphabet', NULL, 2)
208208
----
209209
NULL
210210

211-
statement error Function 'substr' failed to match any signature
211+
statement error DataFusion error: Error during planning: Function 'substr' requires String, but received Int64
212212
SELECT substr(1, 3)
213213

214-
statement error Function 'substr' failed to match any signature
214+
statement error DataFusion error: Error during planning: Function 'substr' requires String, but received Int64
215215
SELECT substr(1, 3, 4)
216216

217217
query T

datafusion/sqllogictest/test_files/named_arguments.slt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ SELECT substr("STR" => 'hello world', "start_pos" => 7);
8686

8787
# Error: wrong number of arguments
8888
# This query provides only 1 argument but substr requires 2 or 3
89-
query error Function 'substr' failed to match any signature
89+
query error DataFusion error: Error during planning: Function 'substr' expects 2 to 3 arguments but received 1
9090
SELECT substr(str => 'hello world');
9191

9292
#############

0 commit comments

Comments
 (0)