gene-bordegaray commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3917238090
##########
datafusion/physical-plan/src/projection.rs:
##########
@@ -1014,6 +1030,10 @@ pub fn remove_unnecessary_projections(
plan: Arc<dyn ExecutionPlan>,
) -> Result<Transformed<Arc<dyn ExecutionPlan>>> {
let maybe_modified = if let Some(projection) =
plan.downcast_ref::<ProjectionExec>() {
+ // Removing a projection with observable metadata can change query
results.
+ if projection.overrides_metadata()? {
+ return Ok(Transformed::no(plan));
+ }
Review Comment:
The cost concern here is solved with the changes that will be mae to
`overrides_metadata` so i think we are good on that frohnt. It will now read
the cached bool
For the condition removal, I don't believe we can do this. There are two
checks that I think are being confused as checking the same thing:
1. In `is_projection_removable` we check if the schemas of the projuection
and its input are a match and if they are we can remove it.
2. Then we check if we can swap the projection to be below its child, like
in the filter example above. This is a differnt check and the guard is still
needed.
Say you had this:
```text
Projection: a metadata={unit: ms}
Filter: arrow_metadata(a, "unit") IS NULL
Input: a,b metadata={}
```
After the swap:
```text
Filter: arrow_metadata(a, "unit") IS NULL
Projection: a metadata={unit: ms}
Input
```
The filter outputs the schema it receives so it now receives the schema with
the metadata. Meaning that the proposed check will pass, but the filter now
also evaluates its predicate using that metadata. Before this it saw rows with
no metadata thus returns NULL and the rows pass. Then after it is inspecting it
with metadata so the rows dont pass.
I don't know if this is accessible via SQL, but it defnitely can via public
APIs. I added a test that checks for this regression.
--
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]
