jayzhan211 opened a new pull request, #25534: URL: https://github.com/apache/datafusion/pull/25534
## Which issue does this PR close? - N/A — refactor, no issue filed. ## Rationale for this change `PartitionedTopK` (`ROW_NUMBER`), `PartitionedTopKRank` (`RANK`) and `PartitionedTopKDenseRank` (`DENSE_RANK`) were added one after another, each as a copy of the previous one. They differ only in the per-partition state and in how one partition's rows are classified, but every copy also repeats: - the same 12 fields (schema, metrics, reservation, both row converters, both scratch `Rows`, the `partition_groups` scratch map, `k`, `batch_size`) and their doc comments, - the same `try_new` body, - the same first three steps of `insert_batch` (encode partition keys, group row indices by key, encode ORDER BY keys), - the same `emit` skeleton (sort partition keys, coalesce, record output, wrap in a stream), - the same `size()` skeleton. That is roughly 150 lines per operator that must be kept in sync by hand, and a fix to one (for example the partition-key byte accounting in `size()`) has to be remembered for the other two. ## What changes are included in this PR? No behaviour change. - Add a private `PartitionedTopKBase` holding the shared fields, with `try_new`, `encode_and_group`, `finish_batch`, `emit` and `size` helpers. - Each operator becomes `base` + its own per-partition map (plus `ob_runs` / `store` for `DENSE_RANK`). Only the per-partition classification loop, the per-partition emit closure and the operator-specific size terms remain in each type. - `PartitionedTopKExec` and the public/crate-visible signatures of the three types are unchanged. Memory consumer names are unchanged. - The per-partition loops are moved verbatim (only `self.scratch_rows` → `self.base.scratch_rows`). - `emit` now sorts `(key, state)` pairs directly instead of cloning every partition key into a separate sorted `Vec` and looking each one up again. Net: +241 / −459 lines in `topk/mod.rs`. ## What is the testing strategy for this PR? Covered by existing tests: the 90 `topk` unit tests (including the exact `size()` reconstruction test for `DENSE_RANK`, updated only for the new field paths), the `window_topn` physical optimizer tests and the window Top-N sqllogictests. ## Are there any user-facing changes? No. -- 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]
