jorgecarleitao commented on a change in pull request #7796:
URL: https://github.com/apache/arrow/pull/7796#discussion_r459018140



##########
File path: rust/datafusion/src/execution/context.rs
##########
@@ -455,12 +495,11 @@ impl ExecutionContext {
         input_schema: &Schema,
     ) -> Result<Arc<dyn PhysicalExpr>> {
         match e {
-            Expr::Alias(expr, name) => {
-                let expr = self.create_physical_expr(expr, input_schema)?;
-                Ok(Arc::new(Alias::new(expr, &name)))
-            }
-            Expr::Column(i) => {
-                Ok(Arc::new(Column::new(*i, &input_schema.field(*i).name())))
+            Expr::Alias(expr, ..) => Ok(self.create_physical_expr(expr, 
input_schema)?),

Review comment:
       (I wrote my other comment because I became uncertain about my claim and 
wanted to double check ^_^: yes, good to have this triple checked; this is a 
big change)
   
   Let's see:
   
   1. the schema of the logical plan is built on the column's names as per 
[this 
line](https://github.com/apache/arrow/pull/7796/files#diff-be9adb93b0effc41a9672892e1aed3e1R1012),
 which is then passed to the physical plan as per line of this discussion. So, 
it should be the case...
   2. [this 
test](https://github.com/apache/arrow/pull/7796/files#diff-8273e76b6910baa123f3a25a967af3b5R1089)
 IMO demonstrates this: we alias an expression in the logical plan, and the 
physical's plan schema's field of that column is the alias itself.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to