gene-bordegaray commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3919761930
##########
datafusion/physical-plan/src/projection.rs:
##########
@@ -1331,12 +1357,20 @@ pub fn update_join_filter(
fn try_collapse_projection_chain(
outer: &ProjectionExec,
) -> Result<Option<Arc<dyn ExecutionPlan>>> {
+ if outer.overrides_metadata()? {
+ return Ok(None);
+ }
Review Comment:
wow this is a cool concept, I haven't heard of this method. Doing right now,
I will report back here
Here is the the case I added with an agent to do many mutations:
```text
Projection output field: i
metadata={"event_field":"true"}
Original child field: i
metadata={}
```
So the projection adds metadata and we check that we override metadata.
Then replaces the child with one that has the same metadata:
```text
Projection output field: i
metadata={"event_field":"true"}
Replacement child field: i
metadata={"event_field":"true"}
```
Now the projection is no longer adding metadata and we check that
`reclaculte_children` switches `override_metadata` to false.
When the implementation to copies the old value:
```rust
let overrides_metadata = self.overrides_metadata;
```
the test fails. But this wasn't covered before
--
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]