jayzhan211 commented on code in PR #25009:
URL: https://github.com/apache/datafusion/pull/25009#discussion_r4047300298
##########
datafusion/physical-plan/src/projection.rs:
##########
@@ -673,20 +673,34 @@ impl ExecutionPlan for ProjectionExec {
metrics: _,
// Derived plan properties, recomputed on decode.
cache: _,
- // Derived metadata comparison, recomputed with the projector.
- overrides_metadata: _,
+ overrides_metadata,
} = self;
let projection_exprs = projector.projection().as_ref();
let input = ctx.encode_child(input)?;
let expr = ctx.encode_expressions(projection_exprs.iter().map(|p|
&p.expr))?;
let expr_name = projection_exprs.iter().map(|p|
p.alias.clone()).collect();
+ let output_schema = projector.output_schema();
+ // Keep inherited metadata self-contained, and retain empty overrides
+ // that explicitly clear metadata from the input.
+ let schema = if *overrides_metadata
Review Comment:
The `|| !metadata.is_empty()` arms encode a full `Schema` for plain
`try_new` projections, whose metadata decode already re-derives from the child.
Measured on a 200-col identity projection with one metadata key per field: the
projection node grows by ~6.9 KB (plan 10,354 → 17,248 bytes), repeated for
every projection in a stack.
`roundtrip_projection_metadata_without_child_metadata` only passes by swapping
the child inside the proto, which encode never produces; `overrides_metadata`
alone fixes #24695 and the explicit-clear case.
Fine as a follow-up
```diff
- let schema = if *overrides_metadata
- || !output_schema.metadata().is_empty()
- || output_schema
- .fields()
- .iter()
- .any(|field| !field.metadata().is_empty())
- {
- Some(output_schema.as_ref().try_into()?)
- } else {
- None
- };
+ // Inherited metadata is re-derived from the child on decode.
+ let schema = overrides_metadata
+ .then(|| projector.output_schema().as_ref().try_into())
+ .transpose()?;
```
--
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]
