alamb commented on code in PR #15743:
URL: https://github.com/apache/datafusion/pull/15743#discussion_r2075226062
##########
datafusion/sql/src/statement.rs:
##########
@@ -710,6 +710,25 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
*statement,
&mut planner_context,
)?;
+
+ //Get inferred data_types from the plan if it is empty in the
prepare statement
+ let data_types = match data_types.is_empty() {
Review Comment:
is there a reason to use a match rather than just if/else?
```suggestion
let data_types = if data_types.is_empty() {
```
...
?
##########
datafusion/sql/src/statement.rs:
##########
@@ -710,6 +710,25 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
*statement,
&mut planner_context,
)?;
+
+ //Get inferred data_types from the plan if it is empty in the
prepare statement
+ let data_types = match data_types.is_empty() {
+ true => {
+ let mut data_types: Vec<DataType> = plan
+ .get_parameter_types()?
+ .iter()
+ .filter_map(|d| match d {
+ (_, Some(v)) => Some(v.clone()),
+ _ => None,
+ })
+ .collect::<Vec<DataType>>();
+ data_types.sort();
+
planner_context.with_prepare_param_data_types(data_types.clone());
+ data_types
+ }
+ false => data_types,
+ };
+
Review Comment:
yes, I agree making the behavior change in a separate PR might be better
Perhaps we could add a method on the `Prepare` struct that would return the
parameter types (or walk over the expressions in the plan calling infer types
if the parameter types are not specified)
https://docs.rs/datafusion/latest/datafusion/logical_expr/struct.Prepare.html#structfield.data_types
So then the test would look like
```rust
#[test]
fn test_infer_types_from_join() {
let sql =
"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
"#
);
// NEW: verify the parameter types
let LogicalPlan::Prepare(prepare) = plan else {
panic!("Expected prepare, got {plan:?}");
};
// call new function to create or infer the datatypes
assert_eq!(prepare.get_or_infer_datatypes(), vec![DataType::Int32]);
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
