zhuqi-lucas opened a new issue, #24478: URL: https://github.com/apache/datafusion/issues/24478
## Is your feature request related to a problem or challenge? When a physical optimizer rule inserts a node below a projection, every ancestor on the path to the root is rebuilt through `replace_children`. `replace_children_if_necessary` short-circuits two cases: identical child pointers, and identical child `PlanProperties` pointers. Inserting a sort satisfies neither, since the child is a new object with new properties, so the rebuild falls into `ChildrenPropertiesMode::Recompute`. `Recompute` is all-or-nothing: it derives the node's properties from scratch. For `ProjectionExec` that includes `EquivalenceGroup::project(mapping)`, which rewrites every expression in every equivalence class through the projection mapping. That particular work is redundant here. A sort changes which orderings hold, not which expressions are equal to one another — `SortExec::compute_properties` clones the input's equivalence properties and only calls `reorder`, which clears and re-adds orderings and never touches the group. So the projection re-derives a group identical to the one it already holds. The cost is paid once per projection above the insertion point, on every pass of the rule, so it grows with projection count times expression size times passes. `EnforceSorting` is the clearest case because it walks the whole plan repeatedly, but `EnforceDistribution` rebuilds the same projections. Measured on a query over a 1191-line view with 38 SELECTs, 117 CASE expressions and 7 joins across 11 tables, ten warm samples per configuration: | | median | | --- | --- | | `EnforceSorting` | 197.8ms | | optimizer rules | 338.1ms | | planning wall | 420.6ms | Roughly half of `EnforceSorting` is this recomputation. ## Describe the solution you'd like Skip the group projection when the child's equivalence group is unchanged. `EquivalenceGroup::project` is a pure function of the group and the mapping, so when both are unchanged the previous result can be reused and only the orderings derived. ## Describe alternatives you've considered A narrow fix local to `ProjectionExec`, comparing the old and new child groups on the `Recompute` path. This is what #24445 does. It needs no new `ChildrenPropertiesMode` variant and leaves the shared path and every other operator untouched, which keeps the blast radius small. The general form would be a third `ChildrenPropertiesMode` — something like "orderings changed, everything else did not" — letting each operator take a cheap path rather than recomputing wholesale. That covers operators beyond `ProjectionExec` and would also allow reusing projected constraints, but it changes the trait contract and every implementor, so it seemed worth separating from the measurement above. Happy to pursue it if that is the direction maintainers prefer. ## Additional context `ProjectionExec` is where the group work is genuinely expensive, because it is the operator that has to *transform* the group through a mapping. Most other operators clone the child's group or clone and augment it, which is much cheaper, so the narrow fix captures most of the available saving on the plans measured here. -- 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]
