Skip to content

Commit

Permalink
fix: Avoid re-wrapping planning errors Err(DataFusionError::Plan) for…
Browse files Browse the repository at this point in the history
… use in plan_datafusion_err (#14000)

* fix: unwrapping Err(DataFusionError::Plan) for use in plan_datafusion_err

* test: add tests for error formatting during planning
  • Loading branch information
avkirilishin authored Jan 5, 2025
1 parent 4c898b4 commit 42dce6e
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 20 deletions.
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

0 comments on commit 42dce6e

Please sign in to comment.