mingmwang commented on code in PR #4192:
URL: https://github.com/apache/arrow-datafusion/pull/4192#discussion_r1021141382
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1145,6 +1145,22 @@ impl Projection {
})
}
+ /// Create a new Projection using the specified output schema
+ pub fn new_from_schema(input: Arc<LogicalPlan>, schema: DFSchemaRef) ->
Self {
+ let expr: Vec<Expr> = schema
+ .fields()
+ .iter()
+ .map(|field| field.qualified_column())
+ .map(Expr::Column)
+ .collect();
+ Self {
+ expr,
+ input,
+ schema,
+ alias: None,
+ }
+ }
Review Comment:
> Your concerns are justified, fix it.
>
> I noticed it before, but I forgot when I was writing the code
>
> BTW, I think we should add alias in Union, how do you think it? We should
remove union alias, and replace with projection. @mingmwang
I think the Alias in Datafusion Union/Projection is more like the
SubqueryAlias in SparkSQL, it is not the Column level Alias Expr. If the
original Union has the alias, you can keep the alias in the Union and no need
to move alias from Union to Projection. But if the Union is eliminated(only 1
input), we should keep the Alias in Projection.
I think besides checking the Union Alias, you also need to check whether we
should add additional Alias in new projection exprs.
````
let new_plan = {
new children len == 1 and there is alias in original Union:
Projection(child, alias)
new children len == 1 input and no alias in original Union
child
new children len > 1
let new_union_schema = from_new_children(new_children)
Union(new_children, new_union_schema, union.alias);
}
if new_plan.schema != union.schema {
// Add new projection for column level Alias maybe
}
````
--
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]
