dqkqd commented on code in PR #18286:
URL: https://github.com/apache/datafusion/pull/18286#discussion_r2501045073
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -633,8 +633,46 @@ impl LogicalPlan {
LogicalPlan::Dml(_) => Ok(self),
LogicalPlan::Copy(_) => Ok(self),
LogicalPlan::Values(Values { schema, values }) => {
- // todo it isn't clear why the schema is not recomputed here
- Ok(LogicalPlan::Values(Values { schema, values }))
+ // We cannot compute the correct schema if we only use values.
+ //
+ // For example, given the following plan:
+ // Projection: col_1, col_2
+ // Values: (Float32(1), Float32(10)), (Float32(100),
Float32(10))
+ //
+ // We wouldn't know about `col_1`, and `col_2` if we only
relied on `values`.
+ // To correctly recompute the new schema, we also need to
retain some information
+ // from the original schema.
+ let new_plan =
LogicalPlanBuilder::values(values.clone())?.build()?;
+
+ let qualified_fields = schema
+ .iter()
+ .zip(new_plan.schema().fields())
+ .map(|((table_ref, old_field), new_field)| {
+ // `old_field`'s data type is unknown but
`new_field`'s is known
+ if old_field.data_type().is_null()
+ && !new_field.data_type().is_null()
+ {
+ let field = old_field
+ .as_ref()
+ .clone()
+ .with_data_type(new_field.data_type().clone());
+ (table_ref.cloned(), Arc::new(field))
Review Comment:
Understood, I will look into this in the weekend.
--
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]
