[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5208: bugfix: fix eval `nullalbe()` in `simplify_exprs`

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #5208:
URL: https://github.com/apache/arrow-datafusion/pull/5208#discussion_r1098512853


##########
datafusion/optimizer/src/simplify_expressions/simplify_exprs.rs:
##########
@@ -59,13 +60,12 @@ impl SimplifyExpressions {
         plan: &LogicalPlan,
         execution_props: &ExecutionProps,
     ) -> Result<LogicalPlan> {
-        // We need to pass down the all schemas within the plan tree to `optimize_expr` in order to
-        // to evaluate expression types. For example, a projection plan's schema will only include
-        // projected columns. With just the projected schema, it's not possible to infer types for
-        // expressions that references non-projected columns within the same project plan or its
-        // children plans.
-        let info = plan
-            .all_schemas()
+        // Pass down the `children merge schema` and `plan schema` to evaluate expression types.
+        // pass all `child schema` and `plan schema` isn't enough, because like `t1 semi join t2 on
+        // on t1.id = t2.id`, each individual schema can't contain all the columns in it.
+        let children_merge_schema = DFSchemaRef::new(merge_schema(plan.inputs()));

Review Comment:
   In some ways, it seems to me that we should *only* be using the children's schemas as the expressions *within* he `LogicalPlan` should be in terms of the plan's inputs (the children's schemas) not the plan's output (`plan.schema()`)



-- 
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: github-unsubscribe@arrow.apache.org

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