Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Avoid re-wrapping planning errors Err(DataFusionError::Plan) for use in plan_datafusion_err #14000

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions datafusion/expr/src/expr_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use crate::{utils, LogicalPlan, Projection, Subquery, WindowFunctionDefinition};
use arrow::compute::can_cast_types;
use arrow::datatypes::{DataType, Field};
use datafusion_common::{
not_impl_err, plan_datafusion_err, plan_err, Column, ExprSchema, Result,
TableReference,
not_impl_err, plan_datafusion_err, plan_err, Column, DataFusionError, ExprSchema,
Result, TableReference,
};
use datafusion_functions_window_common::field::WindowUDFFieldArgs;
use std::collections::HashMap;
Expand Down Expand Up @@ -156,7 +156,10 @@ impl ExprSchemable for Expr {
.map_err(|err| {
plan_datafusion_err!(
"{} {}",
err,
match err {
DataFusionError::Plan(msg) => msg,
err => err.to_string(),
},
utils::generate_signature_error_msg(
func.name(),
func.signature().clone(),
Expand All @@ -181,7 +184,10 @@ impl ExprSchemable for Expr {
.map_err(|err| {
plan_datafusion_err!(
"{} {}",
err,
match err {
DataFusionError::Plan(msg) => msg,
err => err.to_string(),
},
utils::generate_signature_error_msg(
func.name(),
func.signature().clone(),
Expand Down Expand Up @@ -485,7 +491,10 @@ impl Expr {
.map_err(|err| {
plan_datafusion_err!(
"{} {}",
err,
match err {
DataFusionError::Plan(msg) => msg,
err => err.to_string(),
},
utils::generate_signature_error_msg(
fun.name(),
fun.signature(),
Expand All @@ -504,7 +513,10 @@ impl Expr {
data_types_with_window_udf(&data_types, udwf).map_err(|err| {
plan_datafusion_err!(
"{} {}",
err,
match err {
DataFusionError::Plan(msg) => msg,
err => err.to_string(),
},
utils::generate_signature_error_msg(
fun.name(),
fun.signature(),
Expand Down
4 changes: 2 additions & 2 deletions datafusion/optimizer/src/analyzer/type_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,7 +1357,7 @@ mod test {

let err = Projection::try_new(vec![udaf], empty).err().unwrap();
assert!(
err.strip_backtrace().starts_with("Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to MY_AVG function: coercion from [Utf8] to the signature Uniform(1, [Float64]) failed")
err.strip_backtrace().starts_with("Error during planning: Failed to coerce arguments to satisfy a call to MY_AVG function: coercion from [Utf8] to the signature Uniform(1, [Float64]) failed")
);
Ok(())
}
Expand Down Expand Up @@ -1407,7 +1407,7 @@ mod test {
.err()
.unwrap()
.strip_backtrace();
assert!(err.starts_with("Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to avg function: coercion from [Utf8] to the signature Uniform(1, [Int8, Int16, Int32, Int64, UInt8, UInt16, UInt32, UInt64, Float32, Float64]) failed."));
assert!(err.starts_with("Error during planning: Failed to coerce arguments to satisfy a call to avg function: coercion from [Utf8] to the signature Uniform(1, [Int8, Int16, Int32, Int64, UInt8, UInt16, UInt32, UInt64, Float32, Float64]) failed."));
Ok(())
}

Expand Down
52 changes: 52 additions & 0 deletions datafusion/sql/tests/sql_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4500,3 +4500,55 @@ fn test_custom_type_plan() -> Result<()> {

Ok(())
}

fn error_message_test(sql: &str, err_msg_starts_with: &str) {
let err = logical_plan(sql).expect_err("query should have failed");
assert!(
err.strip_backtrace().starts_with(err_msg_starts_with),
"Expected error to start with '{}', but got: '{}'",
err_msg_starts_with,
err.strip_backtrace(),
);
}

#[test]
fn test_error_message_invalid_scalar_function_signature() {
error_message_test(
"select sqrt()",
"Error during planning: sqrt does not support zero arguments",
);
error_message_test(
"select sqrt(1, 2)",
"Error during planning: Failed to coerce arguments",
);
}

#[test]
fn test_error_message_invalid_aggregate_function_signature() {
error_message_test(
"select sum()",
"Error during planning: sum does not support zero arguments",
);
// We keep two different prefixes because they clarify each other.
// 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",
);
}

#[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",
);
}

#[test]
fn test_error_message_invalid_window_aggregate_function_signature() {
error_message_test(
"select sum() over()",
"Error during planning: sum does not support zero arguments",
);
}
12 changes: 6 additions & 6 deletions datafusion/sqllogictest/test_files/aggregate.slt
Original file line number Diff line number Diff line change
Expand Up @@ -76,26 +76,26 @@ statement error DataFusion error: Schema error: Schema contains duplicate unqual
SELECT approx_distinct(c9) count_c9, approx_distinct(cast(c9 as varchar)) count_c9_str FROM aggregate_test_100

# csv_query_approx_percentile_cont_with_weight
statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Utf8, Int8, Float64\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Utf8, Int8, Float64\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont_with_weight(c1, c2, 0.95) FROM aggregate_test_100

statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Int16, Utf8, Float64\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Int16, Utf8, Float64\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont_with_weight(c3, c1, 0.95) FROM aggregate_test_100

statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Int16, Int8, Utf8\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont_with_weight function: coercion from \[Int16, Int8, Utf8\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont_with_weight(c3, c2, c1) FROM aggregate_test_100

# csv_query_approx_percentile_cont_with_histogram_bins
statement error DataFusion error: External error: This feature is not implemented: Tdigest max_size value for 'APPROX_PERCENTILE_CONT' must be UInt > 0 literal \(got data type Int64\)\.
SELECT c1, approx_percentile_cont(c3, 0.95, -1000) AS c3_p95 FROM aggregate_test_100 GROUP BY 1 ORDER BY 1

statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Int16, Float64, Utf8\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Int16, Float64, Utf8\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont(c3, 0.95, c1) FROM aggregate_test_100

statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Int16, Float64, Float64\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Int16, Float64, Float64\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont(c3, 0.95, 111.1) FROM aggregate_test_100

statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Float64, Float64, Float64\] to the signature OneOf(.*) failed(.|\n)*
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to approx_percentile_cont function: coercion from \[Float64, Float64, Float64\] to the signature OneOf(.*) failed(.|\n)*
SELECT approx_percentile_cont(c12, 0.95, 111.1) FROM aggregate_test_100

statement error DataFusion error: This feature is not implemented: Percentile value for 'APPROX_PERCENTILE_CONT' must be a literal
Expand Down
4 changes: 2 additions & 2 deletions datafusion/sqllogictest/test_files/array.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,7 @@ from arrays_values_without_nulls;
## array_element (aliases: array_extract, list_extract, list_element)

# Testing with empty arguments should result in an error
query error DataFusion error: Error during planning: Error during planning: array_element does not support zero arguments
query error DataFusion error: Error during planning: array_element does not support zero arguments
select array_element();

# array_element error
Expand Down Expand Up @@ -2023,7 +2023,7 @@ select array_slice(a, -1, 2, 1), array_slice(a, -1, 2),
[6.0] [6.0] [] []

# Testing with empty arguments should result in an error
query error DataFusion error: Error during planning: Error during planning: array_slice does not support zero arguments
query error DataFusion error: Error during planning: array_slice does not support zero arguments
select array_slice();

## array_any_value (aliases: list_any_value)
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sqllogictest/test_files/errors.slt
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,19 @@ query error
select avg(c1, c12) from aggregate_test_100;

# AggregateFunction with wrong argument type
statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to regr_slope function: coercion from
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to regr_slope function: coercion from
select regr_slope(1, '2');

# WindowFunction using AggregateFunction wrong signature
statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to regr_slope function: coercion from
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to regr_slope function: coercion from
select
c9,
regr_slope(c11, '2') over () as min1
from aggregate_test_100
order by c9

# WindowFunction wrong signature
statement error DataFusion error: Error during planning: Error during planning: Failed to coerce arguments to satisfy a call to nth_value function: coercion from \[Int32, Int64, Int64\] to the signature OneOf\(\[Any\(0\), Any\(1\), Any\(2\)\]\) failed
statement error DataFusion error: Error during planning: Failed to coerce arguments to satisfy a call to nth_value function: coercion from \[Int32, Int64, Int64\] to the signature OneOf\(\[Any\(0\), Any\(1\), Any\(2\)\]\) failed
select
c9,
nth_value(c5, 2, 3) over (order by c9) as nv1
Expand Down
2 changes: 1 addition & 1 deletion datafusion/sqllogictest/test_files/scalar.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1940,7 +1940,7 @@ select position('' in '')
----
1

query error DataFusion error: Error during planning: Error during planning: The signature expected NativeType::String but received NativeType::Int64
query error DataFusion error: Error during planning: The signature expected NativeType::String but received NativeType::Int64
select position(1 in 1)

query I
Expand Down
Loading