From 3f994c59eb6af32e86f167ba666b92cbdf13539f Mon Sep 17 00:00:00 2001 From: overtrue Date: Tue, 11 Aug 2026 23:14:23 +0800 Subject: [PATCH] fix(select): classify function argument planner errors --- crates/s3select-query/src/sql/planner.rs | 16 ++++++- .../src/test/error_handling_test.rs | 47 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/crates/s3select-query/src/sql/planner.rs b/crates/s3select-query/src/sql/planner.rs index 8d3cf4908..a5bbb2881 100644 --- a/crates/s3select-query/src/sql/planner.rs +++ b/crates/s3select-query/src/sql/planner.rs @@ -68,7 +68,7 @@ impl<'a, S: ContextProviderExtension + Send + Sync + 'a> SqlPlanner<'a, S> { match stmt { Statement::Query(_) => { validate_s3_select_statement(&stmt)?; - let df_plan = self.df_planner.sql_statement_to_plan(stmt)?; + let df_plan = self.df_planner.sql_statement_to_plan(stmt).map_err(classify_planner_error)?; let plan = Plan::Query(QueryPlan { df_plan, is_tag_scan: false, @@ -81,6 +81,20 @@ impl<'a, S: ContextProviderExtension + Send + Sync + 'a> SqlPlanner<'a, S> { } } +fn classify_planner_error(error: datafusion::common::DataFusionError) -> QueryError { + if matches!( + &error, + datafusion::common::DataFusionError::Plan(message) + if message.starts_with("Failed to coerce arguments to satisfy a call to") + || (message.starts_with("Internal error: Function '") + && message.contains("' failed to match any signature, errors:")) + ) { + return SelectError::IncorrectSqlFunctionArgumentType.into(); + } + + error.into() +} + fn validate_s3_select_statement(statement: &Statement) -> QueryResult<()> { let Statement::Query(query) = statement else { return Err(unsupported_structure("only SELECT queries are supported")); diff --git a/crates/s3select-query/src/test/error_handling_test.rs b/crates/s3select-query/src/test/error_handling_test.rs index f0300252c..1462719ac 100644 --- a/crates/s3select-query/src/test/error_handling_test.rs +++ b/crates/s3select-query/src/test/error_handling_test.rs @@ -127,6 +127,53 @@ mod error_handling_tests { assert!(matches!(error.select_error(), SelectError::UnsupportedSqlStructure { .. })); } + #[tokio::test] + async fn test_function_argument_coercion_failure_is_typed() { + for sql in ["SELECT ROUND(3.14, 1.1) FROM S3Object", "SELECT SQRT(1, 2) FROM S3Object"] { + let input = create_test_input_with_sql(sql); + let db = get_global_db(input.clone(), true) + .await + .expect("test database should initialize"); + let query = Query::new(Context { input: Arc::new(input) }, sql.to_string()); + + let error = match db.execute(&query).await { + Err(error) => error, + Ok(_) => panic!("invalid function arguments must fail during planning: {sql}"), + }; + + assert_eq!( + error.select_error(), + SelectError::IncorrectSqlFunctionArgumentType, + "unexpected planner error for {sql}: {error:?}" + ); + } + } + + #[tokio::test] + async fn test_other_planner_failures_remain_invalid_query() { + for sql in [ + "SELECT DEFINITELY_UNKNOWN_FUNCTION(1) FROM S3Object", + "SELECT 1 + 'text' FROM S3Object", + ] { + let input = create_test_input_with_sql(sql); + let db = get_global_db(input.clone(), true) + .await + .expect("test database should initialize"); + let query = Query::new(Context { input: Arc::new(input) }, sql.to_string()); + + let error = match db.execute(&query).await { + Err(error) => error, + Ok(_) => panic!("invalid query must fail during planning: {sql}"), + }; + + assert_eq!( + error.select_error(), + SelectError::InvalidQuery, + "unexpected planner error for {sql}: {error:?}" + ); + } + } + #[tokio::test] async fn test_invalid_column_references() { let invalid_column_sqls = vec![