This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new 7ab03e7e23 chore(placeholder): update error message and add tests
(#9073)
7ab03e7e23 is described below
commit 7ab03e7e23eb9630821eaa2ca2325761f3029368
Author: Chunchun Ye <[email protected]>
AuthorDate: Wed Jan 31 13:59:40 2024 -0600
chore(placeholder): update error message and add tests (#9073)
* test(placeholder): add positive test
* test(placeholder): add negative test and update error message
* test(sql_integration): update error message
* test(placeholder): move test from sql to dataframe
chore: add positive test and remove println
---
datafusion/core/tests/dataframe/mod.rs | 74 ++++++++++++++++++++++++++++++++-
datafusion/expr/src/expr_schema.rs | 2 +-
datafusion/sql/tests/sql_integration.rs | 2 +-
3 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/datafusion/core/tests/dataframe/mod.rs
b/datafusion/core/tests/dataframe/mod.rs
index 89ab04dfee..dc347ed9c8 100644
--- a/datafusion/core/tests/dataframe/mod.rs
+++ b/datafusion/core/tests/dataframe/mod.rs
@@ -45,8 +45,9 @@ use datafusion_execution::runtime_env::RuntimeEnv;
use datafusion_expr::expr::{GroupingSet, Sort};
use datafusion_expr::{
array_agg, avg, cast, col, count, exists, expr, in_subquery, lit, max,
out_ref_col,
- scalar_subquery, sum, when, wildcard, AggregateFunction, Expr,
ExprSchemable,
- WindowFrame, WindowFrameBound, WindowFrameUnits, WindowFunctionDefinition,
+ placeholder, scalar_subquery, sum, when, wildcard, AggregateFunction, Expr,
+ ExprSchemable, WindowFrame, WindowFrameBound, WindowFrameUnits,
+ WindowFunctionDefinition,
};
use datafusion_physical_expr::var_provider::{VarProvider, VarType};
@@ -1759,3 +1760,72 @@ async fn test_array_agg() -> Result<()> {
Ok(())
}
+
+#[tokio::test]
+async fn test_dataframe_placeholder_missing_param_values() -> Result<()> {
+ let ctx = SessionContext::new();
+
+ let df = ctx
+ .read_empty()
+ .unwrap()
+ .with_column("a", lit(1))
+ .unwrap()
+ .filter(col("a").eq(placeholder("$0")))
+ .unwrap();
+
+ let logical_plan = df.logical_plan();
+ let formatted = logical_plan.display_indent_schema().to_string();
+ let actual: Vec<&str> = formatted.trim().lines().collect();
+ let expected = vec![
+ "Filter: a = $0 [a:Int32]",
+ " Projection: Int32(1) AS a [a:Int32]",
+ " EmptyRelation []",
+ ];
+ assert_eq!(
+ expected, actual,
+ "\n\nexpected:\n\n{expected:#?}\nactual:\n\n{actual:#?}\n\n"
+ );
+
+ // The placeholder is not replaced with a value,
+ // so the filter data type is not know, i.e. a = $0.
+ // Therefore, the optimization fails.
+ let optimized_plan = ctx.state().optimize(logical_plan);
+ assert!(optimized_plan.is_err());
+ assert!(optimized_plan
+ .unwrap_err()
+ .to_string()
+ .contains("Placeholder type could not be resolved. Make sure that the
placeholder is bound to a concrete type, e.g. by providing parameter values."));
+
+ // Prodiving a parameter value should resolve the error
+ let df = ctx
+ .read_empty()
+ .unwrap()
+ .with_column("a", lit(1))
+ .unwrap()
+ .filter(col("a").eq(placeholder("$0")))
+ .unwrap()
+ .with_param_values(vec![("0", ScalarValue::from(3i32))]) // <--
provide parameter value
+ .unwrap();
+
+ let logical_plan = df.logical_plan();
+ let formatted = logical_plan.display_indent_schema().to_string();
+ let actual: Vec<&str> = formatted.trim().lines().collect();
+ let expected = vec![
+ "Filter: a = Int32(3) [a:Int32]",
+ " Projection: Int32(1) AS a [a:Int32]",
+ " EmptyRelation []",
+ ];
+ assert_eq!(
+ expected, actual,
+ "\n\nexpected:\n\n{expected:#?}\nactual:\n\n{actual:#?}\n\n"
+ );
+
+ let optimized_plan = ctx.state().optimize(logical_plan);
+ assert!(optimized_plan.is_ok());
+
+ let actual = optimized_plan.unwrap().display_indent_schema().to_string();
+ let expected = "EmptyRelation [a:Int32]";
+ assert_eq!(expected, actual);
+
+ Ok(())
+}
diff --git a/datafusion/expr/src/expr_schema.rs
b/datafusion/expr/src/expr_schema.rs
index 4967e66fed..865279ab9a 100644
--- a/datafusion/expr/src/expr_schema.rs
+++ b/datafusion/expr/src/expr_schema.rs
@@ -159,7 +159,7 @@ impl ExprSchemable for Expr {
Expr::Like { .. } | Expr::SimilarTo { .. } =>
Ok(DataType::Boolean),
Expr::Placeholder(Placeholder { data_type, .. }) => {
data_type.clone().ok_or_else(|| {
- plan_datafusion_err!("Placeholder type could not be
resolved")
+ plan_datafusion_err!("Placeholder type could not be
resolved. Make sure that the placeholder is bound to a concrete type, e.g. by
providing parameter values.")
})
}
Expr::Wildcard { qualifier } => {
diff --git a/datafusion/sql/tests/sql_integration.rs
b/datafusion/sql/tests/sql_integration.rs
index 9e6d46a7b3..3c72bf334e 100644
--- a/datafusion/sql/tests/sql_integration.rs
+++ b/datafusion/sql/tests/sql_integration.rs
@@ -465,7 +465,7 @@ Dml: op=[Insert Into] table=[test_decimal]
)]
#[case::placeholder_type_unresolved(
"INSERT INTO person (id, first_name, last_name) VALUES ($2, $4, $6)",
- "Error during planning: Placeholder type could not be resolved"
+ "Error during planning: Placeholder type could not be resolved. Make sure
that the placeholder is bound to a concrete type, e.g. by providing parameter
values."
)]
#[case::placeholder_type_unresolved(
"INSERT INTO person (id, first_name, last_name) VALUES ($id, $first_name,
$last_name)",
