NGA-TRAN commented on code in PR #4561:
URL: https://github.com/apache/arrow-datafusion/pull/4561#discussion_r1044039751
##########
datafusion/core/tests/sql/select.rs:
##########
@@ -1257,6 +1258,73 @@ async fn csv_join_unaliased_subqueries() -> Result<()> {
Ok(())
}
+// Test prepare statement from sql to final result
+// This test is equivalent with the test parallel_query_with_filter below but
using prepare statement
+#[tokio::test]
+async fn test_prepare_statement() -> Result<()> {
Review Comment:
I want to have this test to verify my logical plan works correctly
##########
datafusion/sql/src/planner.rs:
##########
@@ -6311,9 +6308,18 @@ mod tests {
let expected_dt = "[Int32]";
- prepare_stmt_quick_test(sql, expected_plan, expected_dt);
+ let plan = prepare_stmt_quick_test(sql, expected_plan, expected_dt);
+
+ ///////////////////
+ // replace params with values
+ let param_values = vec![ScalarValue::Int32(Some(10))];
+ let expected_plan = "Projection: person.id, person.age\
+ \n Filter: person.age = Int64(10)\
+ \n TableScan: person";
Review Comment:
All the available tests for prepare logical plan are used to replace params
with values
##########
datafusion/sql/src/planner.rs:
##########
@@ -6324,7 +6330,54 @@ mod tests {
let expected_dt = "[]";
- prepare_stmt_quick_test(sql, expected_plan, expected_dt);
+ let plan = prepare_stmt_quick_test(sql, expected_plan, expected_dt);
+
+ ///////////////////
+ // replace params with values
+ let param_values = vec![];
+ let expected_plan = "Projection: person.id, person.age\
+ \n Filter: person.age = Int64(10)\
+ \n TableScan: person";
+
+ prepare_stmt_replace_params_quick_test(plan, param_values,
expected_plan);
+ }
+
+ #[test]
+ #[should_panic(expected = "value: Internal(\"Expected 1 parameters, got
0\")")]
+ fn test_prepare_statement_to_plan_one_param_no_value_panic() {
Review Comment:
And more panics are added for new functions
##########
datafusion/sql/src/planner.rs:
##########
@@ -6239,11 +6256,7 @@ mod tests {
// param is not number following the $ sign
// panic due to error returned from the parser
let sql = "PREPARE my_plan(INT) AS SELECT id, age FROM person WHERE
age = $foo";
-
- let expected_plan = "whatever";
- let expected_dt = "whatever";
-
- prepare_stmt_quick_test(sql, expected_plan, expected_dt);
+ logical_plan(sql).unwrap();
Review Comment:
Instead of calling `prepare_stmt_quick_test` that will first generate
logical plan (and then panic for this test), I call `logical_plan` directly to
simplify the test but still test the same things
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -534,6 +574,655 @@ impl LogicalPlan {
_ => {}
}
}
+
+ /// recursively to replace the params (e.g $1 $2, ...) wit corresponding
values provided in the prams_values
+ pub fn replace_params_with_values(
+ &self,
+ param_values: &Vec<ScalarValue>,
+ ) -> Result<LogicalPlan, DataFusionError> {
Review Comment:
This is actually create a new logical plan. I hope sometime in the future
someone will optimize this to only create expressions for placeholders/params
and keep the rest the same. Not sure if that is possible/easy though
##########
datafusion/core/tests/sql/select.rs:
##########
@@ -1257,6 +1258,73 @@ async fn csv_join_unaliased_subqueries() -> Result<()> {
Ok(())
}
+// Test prepare statement from sql to final result
+// This test is equivalent with the test parallel_query_with_filter below but
using prepare statement
+#[tokio::test]
+async fn test_prepare_statement() -> Result<()> {
+ let tmp_dir = TempDir::new()?;
+ let partition_count = 4;
+ let ctx = partitioned_csv::create_ctx(&tmp_dir, partition_count).await?;
+
+ // sql to statement then to prepare logical plan with parameters
+ // c1 defined as UINT32, c2 defined as UInt64 but the params are Int32 and
Float64
+ let logical_plan =
+ ctx.create_logical_plan("PREPARE my_plan(INT, DOUBLE) AS SELECT c1, c2
FROM test WHERE c1 > $2 AND c1 < $1")?;
+
+ // prepare logical plan to logical plan without parameters
+ let param_values = vec![ScalarValue::Int32(Some(3)),
ScalarValue::Float64(Some(0.0))];
+ let logical_plan = LogicalPlan::execute(logical_plan, param_values)?;
Review Comment:
This is the key test here to explicitly convert prepare logical plan to
normal logical plan
--
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]