Append generated column to the schema instead of prepending for WindowAggExec#4746
Append generated column to the schema instead of prepending for WindowAggExec#4746alamb merged 4 commits intoapache:masterfrom synnada-ai:feature/window_schema_append
Conversation
…nada-ai/arrow-datafusion into feature/window_schema_append # Conflicts: # datafusion/core/src/physical_plan/windows/window_agg_exec.rs # datafusion/core/tests/sql/window.rs
alamb
left a comment
There was a problem hiding this comment.
The code changes look good to me and I carefully reviewed the plan changes. Nice cleanup 👍
cc @mingmwang
|
For some reason github thinks there are conflicts with this PR -- can you please resolve them so I can merge this PR in? |
|
Should be good to good to go, feel free to merge after CI passes. Thank you! |
|
LGTM |
|
Thanks again |
|
Benchmark runs are scheduled for baseline = 41c72cf and contender = ad0459e. ad0459e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
This is mostly a simplification PR. This PR un-does the changes in the #4455
Rationale for this change
Previously
WindowAggExecwas prepending its result to the schema. This required offset addition to columns of the original schema (in the PR #4455). By appending, instead of prepending, we can remove changes in the PR #4455. This decreases code size and increases readability.What changes are included in this PR?
Since column index changes, physical plan of the some queries changed (very minor change hust index). Hence some tests needed to be updated also.
Are these changes tested?
Existing tests checks this functionality, specifically this test checks the desired behavior.
Are there any user-facing changes?