gene-bordegaray commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3854265181
##########
datafusion/physical-plan/src/projection.rs:
##########
@@ -1030,7 +1030,8 @@ pub fn remove_unnecessary_projections(
}
/// Compare the inputs and outputs of the projection. All expressions must be
-/// columns without alias, and projection does not change the order of fields.
+/// columns without alias, the projection must not change the order of fields,
+/// and its output schema must match its input schema, including metadata.
Review Comment:
This was existing behavior but happens because if we removed a projection
that flipped order then we might get wrong results if the ordering was part of
the query like this:
Input plan
```text
DataSourceExec
schema: [value:Int32, value:Int32]
row: [10, 20]
```
The projection then swaps them:
```text
ProjectionExec: expr=[value@1, value@0]
schema: [value:Int32, value:Int32]
row: [20, 10]
DataSourceExec
schema: [value:Int32, value:Int32]
row: [10, 20]
```
The projection stays because the indices do not match their output positions
with that check. So we get the right result: `[20, 10]`
If we removed it then the names and schemas match, so the optimizer could
remove the projection:
```text
DataSourceExec
row: [10, 20]
```
So [20, 10] order gets lost.
--
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]
