mustafasrepo commented on code in PR #9690:
URL: https://github.com/apache/arrow-datafusion/pull/9690#discussion_r1531680604


##########
datafusion/optimizer/src/optimize_projections.rs:
##########
@@ -162,14 +163,37 @@ fn optimize_projections(
                 .map(|input| 
((0..input.schema().fields().len()).collect_vec(), false))
                 .collect::<Vec<_>>()
         }
+        LogicalPlan::Extension(extension) => {
+            let children = extension.node.inputs();
+            if children.len() != 1 {
+                // TODO: Add support for `LogicalPlan::Extension` with multi 
children.
+                return Ok(None);
+            }
+            // has single child
+            let exprs = plan.expressions();
+            let child = children[0];
+            let node_schema = extension.node.schema();
+            let child_schema = child.schema();
+            if let Some(parent_required_indices_mapped) =

Review Comment:
   > I suggest for this PR, only push down information about used expressions 
(don't try and incorporate a projection from the parent) and we can handle the 
parent projection information in a follow on PR.
   This behavior may not be safe. Assuming a user defined operator similar to 
`LogicalPlan::Filter`. Following Plan
   ```
   Projection(a,b,c)
   --UserDefineFilter(d=0)
   ----TableScan(a,b,c,d)
   ```
   If we don't incorporate requirements from the parent, user defined filter 
may insert `Projection(d)` below it. Which would produce following invalid plan:
   ```
   Projection(a,b,c) (a,b,c is missing at its input)
   --UserDefineFilter(d=0)
   ----Projection(d)
   ------TableScan(a,b,c,d)
   ```
   



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

Reply via email to