LiaCastaneda commented on code in PR #15438:
URL: https://github.com/apache/datafusion/pull/15438#discussion_r2025709682
##########
datafusion/physical-expr/src/equivalence/projection.rs:
##########
@@ -66,9 +66,9 @@ impl ProjectionMapping {
let idx = col.index();
let matching_input_field = input_schema.field(idx);
if col.name() != matching_input_field.name() {
- return internal_err!("Input field name {} does
not match with the projection expression {}",
- matching_input_field.name(),col.name())
- }
+ let fixed_col = Column::new(col.name(), idx);
+ return
Ok(Transformed::yes(Arc::new(fixed_col)))
+ }
Review Comment:
Hi, sorry about this - I'm trying to understand this, in that change what I
did is to ammend the issue by unifying the naming instead of skipping the check.
I checked on how we build the LogicalPlan and input fields have the same
names as the projection column expressions, both have count(Int64(1)):1 so I
don't know where that field is being set to count(Int64(1)) for the physcial
input_schema. On the physical planner I also printed the Logical input schema
[here](https://github.com/apache/datafusion/blob/28451b5ff1a1363c3495dc328cf8aaf9bc7b925e/datafusion/core/src/physical_planner.rs#L2009)
and it appears as count(Int64(1)):1 . I also noticed that same function
[creates a
physical](https://github.com/apache/datafusion/blob/28451b5ff1a1363c3495dc328cf8aaf9bc7b925e/datafusion/core/src/physical_planner.rs#L2042)
Expr for the projection based on the logical input_schema and not the physical
input_exec schema hence why it fails later on the check.
I think an approach could be to move the check
[here](https://github.com/apache/datafusion/blob/28451b5ff1a1363c3495dc328cf8aaf9bc7b925e/datafusion/core/src/physical_planner.rs#L2028)
and add an option on the runtime config we can get thorugh the session_state
to skip it (and by default set it to false) so errors can still be caught while
developing (iiuc this doesn't cause errors during execution), something similar
was done for
[aggregate](https://github.com/apache/datafusion/blob/28451b5ff1a1363c3495dc328cf8aaf9bc7b925e/datafusion/core/src/physical_planner.rs#L658)
nodes. I was looking into your PR that adds that check, but apparently we
don't have `create_physical_name` anymore after #11977
--
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]
