Phoenix500526 commented on PR #23780: URL: https://github.com/apache/datafusion/pull/23780#issuecomment-5076143807
> One request: **consider keeping the explanatory comments from #23496.** The port strips them, and a few are rationale rather than narration: > > * The note on `input_order_mode: None` in `WindowAggExec::try_to_proto` — that both executors > share the single `PhysicalPlanType::Window` variant and are told apart _only_ by the presence of > `input_order_mode`. This is the non-obvious invariant of the whole file; someone "tidying up" > that field later would silently break `BoundedWindowAggExec` decoding. > * The mirroring note on the encode side of `BoundedWindowAggExec`, that `Some(input_order_mode)` > is what routes the shared decode arm back to the bounded exec. > * The `WindowUDFExpr::args()` comment explaining that it returns the full, unfiltered argument > list so every argument survives the round-trip (as opposed to `window_expr.expressions()`). > * The `// TODO: Remove extended_schema if functions are all UDAF` marker that was carried in the > original `datafusion-proto` code — that one is pre-existing and shouldn't be dropped as part of > a move. Done -- 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]
