qstommyshu commented on code in PR #15743:
URL: https://github.com/apache/datafusion/pull/15743#discussion_r2083261051
##########
datafusion/sql/src/statement.rs:
##########
@@ -710,6 +710,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
*statement,
&mut planner_context,
)?;
+
Review Comment:
Please remove this empty line
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4770,32 +4717,19 @@ fn test_update_infer() {
("$2".to_string(), Some(DataType::UInt32)),
]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(42)),
ScalarValue::UInt32(Some(1))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Dml: op=[Update] table=[person]
- Projection: person.id AS id, person.first_name AS first_name,
person.last_name AS last_name, Int32(42) AS age, person.state AS state,
person.salary AS salary, person.birth_date AS birth_date, person.😀 AS 😀
- Filter: person.id = UInt32(1)
- TableScan: person
- "
- );
Review Comment:
and here
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4607,82 +4607,58 @@ fn test_prepare_statement_to_plan_params_as_constants()
{
}
#[test]
-fn test_infer_types_from_join() {
+fn test_prepare_statement_infer_types_from_join() {
let sql =
- "SELECT id, order_id FROM person JOIN orders ON id = customer_id and
age = $1";
+ "PREPARE my_plan AS SELECT id, order_id FROM person JOIN orders ON id
= customer_id and age = $1";
let plan = logical_plan(sql).unwrap();
assert_snapshot!(
plan,
@r#"
- Projection: person.id, orders.order_id
- Inner Join: Filter: person.id = orders.customer_id AND person.age = $1
- TableScan: person
- TableScan: orders
+ Prepare: "my_plan" []
+ Projection: person.id, orders.order_id
+ Inner Join: Filter: person.id = orders.customer_id AND person.age = $1
+ TableScan: person
+ TableScan: orders
"#
);
let actual_types = plan.get_parameter_types().unwrap();
let expected_types = HashMap::from([("$1".to_string(),
Some(DataType::Int32))]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(10))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, orders.order_id
- Inner Join: Filter: person.id = orders.customer_id AND person.age =
Int32(10)
- TableScan: person
- TableScan: orders
- "
- );
}
#[test]
-fn test_infer_types_from_predicate() {
- let sql = "SELECT id, age FROM person WHERE age = $1";
+fn test_prepare_statement_infer_types_from_predicate() {
+ let sql = "PREPARE my_plan AS SELECT id, age FROM person WHERE age = $1";
let plan = logical_plan(sql).unwrap();
assert_snapshot!(
plan,
@r#"
- Projection: person.id, person.age
- Filter: person.age = $1
- TableScan: person
+ Prepare: "my_plan" []
+ Projection: person.id, person.age
+ Filter: person.age = $1
+ TableScan: person
"#
);
let actual_types = plan.get_parameter_types().unwrap();
let expected_types = HashMap::from([("$1".to_string(),
Some(DataType::Int32))]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(10))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, person.age
- Filter: person.age = Int32(10)
- TableScan: person
- "
- );
Review Comment:
Same for here
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4607,82 +4607,58 @@ fn test_prepare_statement_to_plan_params_as_constants()
{
}
#[test]
-fn test_infer_types_from_join() {
+fn test_prepare_statement_infer_types_from_join() {
let sql =
- "SELECT id, order_id FROM person JOIN orders ON id = customer_id and
age = $1";
+ "PREPARE my_plan AS SELECT id, order_id FROM person JOIN orders ON id
= customer_id and age = $1";
let plan = logical_plan(sql).unwrap();
assert_snapshot!(
plan,
@r#"
- Projection: person.id, orders.order_id
- Inner Join: Filter: person.id = orders.customer_id AND person.age = $1
- TableScan: person
- TableScan: orders
+ Prepare: "my_plan" []
+ Projection: person.id, orders.order_id
+ Inner Join: Filter: person.id = orders.customer_id AND person.age = $1
+ TableScan: person
+ TableScan: orders
"#
);
let actual_types = plan.get_parameter_types().unwrap();
let expected_types = HashMap::from([("$1".to_string(),
Some(DataType::Int32))]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(10))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, orders.order_id
- Inner Join: Filter: person.id = orders.customer_id AND person.age =
Int32(10)
- TableScan: person
- TableScan: orders
- "
- );
Review Comment:
Great! I see your inferred type changes is in another issue. This part of
the test shouldn't be removed in this issue, right? You can add new inferred
type tests in #16019 along with your changes
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4692,75 +4668,46 @@ fn test_infer_types_from_between_predicate() {
("$2".to_string(), Some(DataType::Int32)),
]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(10)),
ScalarValue::Int32(Some(30))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, person.age
- Filter: person.age BETWEEN Int32(10) AND Int32(30)
- TableScan: person
- "
- );
Review Comment:
and here
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4692,75 +4668,46 @@ fn test_infer_types_from_between_predicate() {
("$2".to_string(), Some(DataType::Int32)),
]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::Int32(Some(10)),
ScalarValue::Int32(Some(30))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, person.age
- Filter: person.age BETWEEN Int32(10) AND Int32(30)
- TableScan: person
- "
- );
}
#[test]
-fn test_infer_types_subquery() {
- let sql = "SELECT id, age FROM person WHERE age = (select max(age) from
person where id = $1)";
+fn test_prepare_statement_infer_types_subquery() {
+ let sql = "PREPARE my_plan AS SELECT id, age FROM person WHERE age =
(select max(age) from person where id = $1)";
let plan = logical_plan(sql).unwrap();
assert_snapshot!(
plan,
@r#"
- Projection: person.id, person.age
- Filter: person.age = (<subquery>)
- Subquery:
- Projection: max(person.age)
- Aggregate: groupBy=[[]], aggr=[[max(person.age)]]
- Filter: person.id = $1
- TableScan: person
- TableScan: person
+ Prepare: "my_plan" []
+ Projection: person.id, person.age
+ Filter: person.age = (<subquery>)
+ Subquery:
+ Projection: max(person.age)
+ Aggregate: groupBy=[[]], aggr=[[max(person.age)]]
+ Filter: person.id = $1
+ TableScan: person
+ TableScan: person
"#
);
let actual_types = plan.get_parameter_types().unwrap();
let expected_types = HashMap::from([("$1".to_string(),
Some(DataType::UInt32))]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![ScalarValue::UInt32(Some(10))];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
-
- assert_snapshot!(
- plan_with_params,
- @r"
- Projection: person.id, person.age
- Filter: person.age = (<subquery>)
- Subquery:
- Projection: max(person.age)
- Aggregate: groupBy=[[]], aggr=[[max(person.age)]]
- Filter: person.id = UInt32(10)
- TableScan: person
- TableScan: person
- "
- );
Review Comment:
and here
##########
datafusion/sql/tests/sql_integration.rs:
##########
@@ -4806,22 +4740,6 @@ fn test_insert_infer() {
("$3".to_string(), Some(DataType::Utf8)),
]);
assert_eq!(actual_types, expected_types);
-
- // replace params with values
- let param_values = vec![
- ScalarValue::UInt32(Some(1)),
- ScalarValue::from("Alan"),
- ScalarValue::from("Turing"),
- ];
- let plan_with_params = plan.with_param_values(param_values).unwrap();
- assert_snapshot!(
- plan_with_params,
- @r#"
- Dml: op=[Insert Into] table=[person]
- Projection: column1 AS id, column2 AS first_name, column3 AS last_name,
CAST(NULL AS Int32) AS age, CAST(NULL AS Utf8) AS state, CAST(NULL AS Float64)
AS salary, CAST(NULL AS Timestamp(Nanosecond, None)) AS birth_date, CAST(NULL
AS Int32) AS 😀
- Values: (UInt32(1) AS $1, Utf8("Alan") AS $2, Utf8("Turing") AS $3)
- "#
- );
Review Comment:
and here
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
