apache / apache/datafusion

Enable TopK per Partition (`datafusion.optimizer.enable_window_topn`) by default

Open
#24,608 0 comments 3 reactions 0 assignees View on GitHub
enhancement EPIC
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.