Enable TopK per Partition (`datafusion.optimizer.enable_window_topn`) by default
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Is your feature request related to a problem or challenge?
While writing the DataFusion 55.0.0 blog post
- https://github.com/apache/datafusion/issues/24216
- https://github.com/apache/datafusion-site/pull/203
I wrote up the per-partition TopK window optimization as one of the headline performance improvements of the release — and only then discovered that `datafusion.optimizer.enable_window_topn` **defaults to `false`**, so nobody actually gets it unless they opt in.
We probably shouldn't make a big deal about a feature that is off by default, and I could not find any issue tracking turning it on. So I am filing this issue so we don't forget and we can track what is required to do so in one place rather than scattered across PR review threads.
#### The story we want to be able to tell
This is the text I had drafted for the 55.0.0 blog post before pulling it, included so it's clear what we're aiming for and so whoever finishes this work can reuse it:
> ### Per-Partition TopK for Window Functions
>
> A common analytics pattern selects the top N rows per group using a window
> function:
>
> ```sql
> SELECT * FROM (
> SELECT ROW_NUMBER() OVER (PARTITION BY category ORDER BY revenue DESC) AS rn, *
> FROM sales
> ) WHERE rn <= 5;
> ```
>
> DataFusion previously sorted the *entire* input to evaluate the window
> function, even though only a handful of rows per partition survive the filter.
> DataFusion 55 recognizes this pattern and uses a new `PartitionedTopKExec`
> operator that keeps only the top N rows per partition, dramatically reducing
> sorting and memory for high-cardinality inputs. The optimization applies to
> `ROW_NUMBER` and `RANK`, resolving a feature request first filed in 2023
> (#6899).
> Thanks to @SubhamSinghal for implementing this feature, with reviews from
> @2010YOUY01 and @kosiew. Related PRs: #21479, #22885, #23096
#### Why it is currently off
`datafusion/common/src/config.rs`:
https://github.com/apache/datafusion/blob/fdf7935a640e557147d5a22268b237021c2d904b/datafusion/common/src/config.rs#L1574-L1581
The default was set to `false` in #21479 because of severe high-cardinality regressions against the plain-sort baseline:
> If it has regressions as large as `0.03x` it should [be] off by default (and
> we should look if we can automatically enable it via a heuristic / stats based
> on partition cardinality / rows)
> — @Dandandan, https://github.com/apache/datafusion/pull/21479#issuecomment-4221809099
#23096 (in 55.0.0) closed most of that gap by sharing the encoder and memory
reservation across partitions:
| Partitions | `main` | with #23096 | vs sort baseline |
|---|---:|---:|---|
| 100 | 110 ms | 105 ms | ~1.0x |
| 1,000 | 117 ms | 110 ms | ~1.0x |
| 10,000 | 640 ms | 137 ms | **1.7x faster than sort** (was a regression) |
| 100,000 | 4,327 ms | 320 ms | 320 ms vs 238 ms — still slower |
but explicitly did not flip the default:
> `enable_window_topn` default stays `false` per the #21479 discussion — 100K+
> remains slower than sort on average, so this PR doesn't motivate flipping the
> default. It's the prerequisite for further optimizations that would attack the
> residual 100K+ cliff.
> — #23096
### Describe the solution you'd like
Set `enable_window_topn` to `true` by default, once we are confident it does not
regress. Known work items:
- [ ] **Close the high-cardinality cliff.** -- @Dandandan's original suggestion in #21479.
- [ ] **Fix RANK memory blowup** — #24591
- [ ] **Fix wide-payload OOM** — #23600
- [ ] **Expose metrics** — #24470 / #24495
- [ ] **Broaden pattern coverage** — #21596 / #23599
- [ ] **Benchmark coverage in CI**: #24050 added a `dense_rank` benchmark; we should make sure the sweep covers high partition-cardinality as well
- [ ] Flip the default, run the full benchmark suite, and update `docs/source/user-guide/configs.md`.
### Describe alternatives you've considered
**Leave it opt-in indefinitely.** 👎 -- an optimization nobody turns on is close to an optimization that doesn't exist
### Additional context
- #24404 (panic on `WHERE rn < 1`) was fixed in #24405, but that landed after the `branch-55` cut
- #6899 (original 2023 request),
- #21479, #22885, #23096, #23355, #24191, #23600, #24470, #24591.
Contributor guide
Research direction
Start with datafusion/common/src/config.rs and review the linked work items for high-cardinality performance, RANK memory use, wide-payload OOMs, metrics, and pattern coverage. The work is done when the default is flipped, the full benchmark suite covers the relevant cardinalities, and docs/source/user-guide/configs.md is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, sql
- Domain
- data-engineering, databases, performance
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100