alamb commented on code in PR #14511:
URL: https://github.com/apache/datafusion/pull/14511#discussion_r1943029692
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -170,26 +170,34 @@ pub fn data_types(
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade:
https://github.com/apache/datafusion/issues/13763
return plan_err!(
- "signature {:?} does not support zero arguments. Use
TypeSignature::Nullary for zero arguments.",
+ "For function {} signature {:?} does not support zero
arguments. Use TypeSignature::Nullary for zero arguments.",
Review Comment:
I found this wording somewhat akward -- maybe something like
```suggestion
"function {} has signature {:?} which does not support zero
arguments. Use TypeSignature::Nullary for zero arguments.",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -257,7 +265,11 @@ fn get_valid_types_with_scalar_udf(
match signature {
TypeSignature::UserDefined => match func.coerce_types(current_types) {
Ok(coerced_types) => Ok(vec![coerced_types]),
- Err(e) => exec_err!("User-defined coercion failed with {:?}", e),
+ Err(e) => exec_err!(
+ "For function {} user-defined coercion failed with {:?}",
Review Comment:
```suggestion
"Function {} user-defined coercion failed with {:?}",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -276,14 +288,15 @@ fn get_valid_types_with_scalar_udf(
// Every signature failed, return the joined error
if res.is_empty() {
internal_err!(
- "Failed to match any signature, errors: {}",
+ "For function {} failed to match any signature, errors:
{}",
Review Comment:
```suggestion
"Function {} failed to match any signature, errors: {}",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -170,26 +170,34 @@ pub fn data_types(
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade:
https://github.com/apache/datafusion/issues/13763
return plan_err!(
- "signature {:?} does not support zero arguments. Use
TypeSignature::Nullary for zero arguments.",
+ "For function {} signature {:?} does not support zero
arguments. Use TypeSignature::Nullary for zero arguments.",
+ function_name.as_ref(),
type_signature
);
} else {
return plan_err!(
- "signature {:?} does not support zero arguments.",
+ "For function {} signature {:?} does not support zero
arguments.",
Review Comment:
```suggestion
"Function {} has signature {:?} which does not support zero
arguments.",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -295,7 +308,13 @@ fn get_valid_types_with_aggregate_udf(
let valid_types = match signature {
TypeSignature::UserDefined => match func.coerce_types(current_types) {
Ok(coerced_types) => vec![coerced_types],
- Err(e) => return exec_err!("User-defined coercion failed with
{:?}", e),
+ Err(e) => {
+ return exec_err!(
+ "For function {} user-defined coercion failed with {:?}",
Review Comment:
```suggestion
"Function {} user-defined coercion failed with {:?}",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -437,10 +466,14 @@ fn get_valid_types(
}
}
- fn function_length_check(length: usize, expected_length: usize) ->
Result<()> {
+ fn function_length_check(
+ function_name: &str,
+ length: usize,
+ expected_length: usize,
+ ) -> Result<()> {
if length != expected_length {
return plan_err!(
- "The signature expected {expected_length} arguments but
received {length}"
+ "For function {function_name} the signature expected
{expected_length} arguments but received {length}"
Review Comment:
```suggestion
"Function {function_name} expects {expected_length}
arguments but received {length}"
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -393,7 +422,7 @@ fn get_valid_types(
let new_base_type = new_base_type.ok_or_else(|| {
internal_datafusion_err!(
- "Coercion from {array_base_type:?} to {elem_base_type:?} not
supported."
+ "For function {function_name} coercion from
{array_base_type:?} to {elem_base_type:?} not supported."
Review Comment:
```suggestion
"Function {function_name} does not support coercion from
{array_base_type:?} to {elem_base_type:?} ."
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -464,30 +497,31 @@ fn get_valid_types(
new_types.push(DataType::Utf8);
} else {
return plan_err!(
- "The signature expected NativeType::String but
received {logical_data_type}"
+ "For function {function_name} the signature expected
NativeType::String but received {logical_data_type}"
Review Comment:
```suggestion
"Function {function_name} expects NativeType::String
but received {logical_data_type}"
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -546,20 +578,20 @@ fn get_valid_types(
valid_type = DataType::Float64;
} else if !logical_data_type.is_numeric() {
return plan_err!(
- "The signature expected NativeType::Numeric but received
{logical_data_type}"
+ "For function {function_name} the signature expected
NativeType::Numeric but received {logical_data_type}"
Review Comment:
```suggestion
"Function {function_name} expects NativeType::Numeric
but received {logical_data_type}"
```
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4536,15 +4536,15 @@ fn
test_error_message_invalid_aggregate_function_signature() {
// It might be incorrect, and we should consider keeping only one.
error_message_test(
"select max(9, 3)",
- "Error during planning: Execution error: User-defined coercion failed",
+ "Error during planning: Execution error: For function max user-defined
coercion failed",
Review Comment:
```suggestion
"Error during planning: Execution error: Function max user-defined
coercion failed",
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -318,25 +337,33 @@ fn get_valid_types_with_window_udf(
let valid_types = match signature {
TypeSignature::UserDefined => match func.coerce_types(current_types) {
Ok(coerced_types) => vec![coerced_types],
- Err(e) => return exec_err!("User-defined coercion failed with
{:?}", e),
+ Err(e) => {
+ return exec_err!(
+ "For function {} user-defined coercion failed with {:?}",
Review Comment:
```suggestion
"Function {} user-defined coercion failed with {:?}",
```
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -571,7 +571,7 @@ select repeat('-1.2', arrow_cast(3, 'Int32'));
----
-1.2-1.2-1.2
-query error DataFusion error: Error during planning: Internal error: Expect
TypeSignatureClass::Native\(LogicalType\(Native\(Int64\), Int64\)\) but
received Float64
+query error DataFusion error: Error during planning: Internal error: For
function repeat expect
TypeSignatureClass::Native\(LogicalType\(Native\(Int64\), Int64\)\) but
received Float64
Review Comment:
```suggestion
query error DataFusion error: Error during planning: Internal error:
Function repeat expects
TypeSignatureClass::Native\(LogicalType\(Native\(Int64\), Int64\)\) but
received Float64
```
##########
datafusion/sqllogictest/test_files/math.slt:
##########
@@ -126,15 +126,15 @@ statement error
SELECT abs(1, 2);
# abs: unsupported argument type
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
Review Comment:
```suggestion
query error DataFusion error: Error during planning: Function abs expects
NativeType::Numeric but received NativeType::String
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -624,7 +659,7 @@ fn get_valid_types(
Ok(current_type.to_owned())
}
_ => {
- not_impl_err!("Got logical_type: {logical_type} with
target_type_class: {target_type_class}")
+ not_impl_err!("For function {function_name} got
logical_type: {logical_type} with target_type_class: {target_type_class}")
Review Comment:
```suggestion
not_impl_err!("Function {function_name} got
logical_type: {logical_type} with target_type_class: {target_type_class}")
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -651,13 +686,13 @@ fn get_valid_types(
}
TypeSignature::UserDefined => {
return internal_err!(
- "User-defined signature should be handled by function-specific
coerce_types."
+ "For function {function_name} user-defined signature should be
handled by function-specific coerce_types."
Review Comment:
```suggestion
"Function {function_name} user-defined signature should be
handled by function-specific coerce_types."
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1042,28 +1085,34 @@ mod tests {
// Cannot coerce args to a common numeric type.
let got = get_valid_types(
+ "test",
&TypeSignature::Numeric(2),
&[DataType::Int32, DataType::Utf8],
)
.unwrap_err();
assert_contains!(
got.to_string(),
- "The signature expected NativeType::Numeric but received
NativeType::String"
+ "For function test the signature expected NativeType::Numeric but
received NativeType::String"
);
// Fallbacks to float64 if the arg is of type null.
- let got = get_valid_types_flatten(&TypeSignature::Numeric(1),
&[DataType::Null]);
+ let got = get_valid_types_flatten(
+ "test",
+ &TypeSignature::Numeric(1),
+ &[DataType::Null],
+ );
assert_eq!(got, [DataType::Float64]);
// Rejects non-numeric arg.
let got = get_valid_types(
+ "test",
&TypeSignature::Numeric(1),
&[DataType::Timestamp(TimeUnit::Second, None)],
)
.unwrap_err();
assert_contains!(
got.to_string(),
- "The signature expected NativeType::Numeric but received
NativeType::Timestamp(Second, None)"
+ "For function test the signature expected NativeType::Numeric but
received NativeType::Timestamp(Second, None)"
Review Comment:
```suggestion
"Function test expected NativeType::Numeric but received
NativeType::Timestamp(Second, None)"
```
##########
datafusion/sqllogictest/test_files/math.slt:
##########
@@ -126,15 +126,15 @@ statement error
SELECT abs(1, 2);
# abs: unsupported argument type
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
SELECT abs('foo');
# abs: numeric string
# TODO: In Postgres, '-1.2' is unknown type and interpreted to float8 so they
don't fail on this query
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
select abs('-1.2');
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
Review Comment:
```suggestion
query error DataFusion error: Error during planning: Function abs expected
NativeType::Numeric but received NativeType::String
```
##########
datafusion/sqllogictest/test_files/scalar.slt:
##########
@@ -1945,7 +1945,7 @@ select position('' in '')
----
1
-query error DataFusion error: Error during planning: The signature expected
NativeType::String but received NativeType::Int64
+query error DataFusion error: Error during planning: For function strpos the
signature expected NativeType::String but received NativeType::Int64
Review Comment:
```suggestion
query error DataFusion error: Error during planning: Function strpos
expected NativeType::String but received NativeType::Int64
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -523,17 +557,15 @@ fn get_valid_types(
if !logical_data_type.is_numeric() {
return plan_err!(
- "The signature expected NativeType::Numeric but
received {logical_data_type}"
+ "For function {function_name} the signature expected
NativeType::Numeric but received {logical_data_type}"
Review Comment:
```suggestion
"Function {function_name} expects
NativeType::Numeric but received {logical_data_type}"
```
##########
datafusion/sqllogictest/test_files/math.slt:
##########
@@ -126,15 +126,15 @@ statement error
SELECT abs(1, 2);
# abs: unsupported argument type
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
SELECT abs('foo');
# abs: numeric string
# TODO: In Postgres, '-1.2' is unknown type and interpreted to float8 so they
don't fail on this query
-query error DataFusion error: Error during planning: The signature expected
NativeType::Numeric but received NativeType::String
+query error DataFusion error: Error during planning: For function abs the
signature expected NativeType::Numeric but received NativeType::String
Review Comment:
```suggestion
query error DataFusion error: Error during planning: Function abs expects
NativeType::Numeric but received NativeType::String
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -464,30 +497,31 @@ fn get_valid_types(
new_types.push(DataType::Utf8);
} else {
return plan_err!(
- "The signature expected NativeType::String but
received {logical_data_type}"
+ "For function {function_name} the signature expected
NativeType::String but received {logical_data_type}"
);
}
}
// Find the common string type for the given types
fn find_common_type(
+ function_name: &str,
lhs_type: &DataType,
rhs_type: &DataType,
) -> Result<DataType> {
match (lhs_type, rhs_type) {
(DataType::Dictionary(_, lhs), DataType::Dictionary(_,
rhs)) => {
- find_common_type(lhs, rhs)
+ find_common_type(function_name, lhs, rhs)
}
(DataType::Dictionary(_, v), other)
- | (other, DataType::Dictionary(_, v)) =>
find_common_type(v, other),
+ | (other, DataType::Dictionary(_, v)) => {
+ find_common_type(function_name, v, other)
+ }
_ => {
if let Some(coerced_type) = string_coercion(lhs_type,
rhs_type) {
Ok(coerced_type)
} else {
plan_err!(
- "{} and {} are not coercible to a common
string type",
- lhs_type,
- rhs_type
+ "For function {function_name}: {lhs_type} and
{rhs_type} are not coercible to a common string type"
Review Comment:
```suggestion
"Function {function_name}: could not coerce
{lhs_type} and {rhs_type} to a common string type"
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -651,13 +686,13 @@ fn get_valid_types(
}
TypeSignature::UserDefined => {
return internal_err!(
- "User-defined signature should be handled by function-specific
coerce_types."
+ "For function {function_name} user-defined signature should be
handled by function-specific coerce_types."
)
}
TypeSignature::VariadicAny => {
if current_types.is_empty() {
return plan_err!(
- "The function expected at least one argument but received
0"
+ "The function {function_name} expected at least one
argument but received 0"
Review Comment:
```suggestion
"Function {function_name} expected at least one argument
but received 0"
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1042,28 +1085,34 @@ mod tests {
// Cannot coerce args to a common numeric type.
let got = get_valid_types(
+ "test",
&TypeSignature::Numeric(2),
&[DataType::Int32, DataType::Utf8],
)
.unwrap_err();
assert_contains!(
got.to_string(),
- "The signature expected NativeType::Numeric but received
NativeType::String"
+ "For function test the signature expected NativeType::Numeric but
received NativeType::String"
Review Comment:
```suggestion
"Function test expected NativeType::Numeric but received
NativeType::String"
```
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4536,15 +4536,15 @@ fn
test_error_message_invalid_aggregate_function_signature() {
// It might be incorrect, and we should consider keeping only one.
error_message_test(
"select max(9, 3)",
- "Error during planning: Execution error: User-defined coercion failed",
+ "Error during planning: Execution error: For function max user-defined
coercion failed",
);
}
#[test]
fn test_error_message_invalid_window_function_signature() {
error_message_test(
"select rank(1) over()",
- "Error during planning: The function expected zero argument but
received 1",
+ "Error during planning: The function rank expected zero argument but
received 1",
Review Comment:
```suggestion
"Error during planning: Function rank expected zero argument but
received 1",
```
--
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]
