You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/16 07:04:39 UTC
(datafusion) branch gh-readonly-queue/main/pr-24405-c1b39bd503cefcc170748216495191b3cdf567ae created (now b551ba0375)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-24405-c1b39bd503cefcc170748216495191b3cdf567ae
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at b551ba0375 fix: skip WindowTopN rewrite when fetch is 0 to avoid PartitionedTopK panic (#24405)
This branch includes the following new commits:
new b551ba0375 fix: skip WindowTopN rewrite when fetch is 0 to avoid PartitionedTopK panic (#24405)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: skip WindowTopN rewrite when fetch is 0 to avoid PartitionedTopK panic (#24405)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24405-c1b39bd503cefcc170748216495191b3cdf567ae
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit b551ba03753d719ff83a14b7f8f36bf667cdee67
Author: Liang-Chi Hsieh <vi...@gmail.com>
AuthorDate: Sun Aug 16 07:04:16 2026 +0000
fix: skip WindowTopN rewrite when fetch is 0 to avoid PartitionedTopK panic (#24405)
## Which issue does this PR close?
- Closes #24404.
## Rationale for this change
With `datafusion.optimizer.enable_window_topn = true`, a query filtering
a partitioned `ROW_NUMBER()`/`RANK()` with `rn < 1` (or the flipped `1 >
rn`) panics with `PartitionedTopK requires k > 0`.
`ROW_NUMBER`/`RANK` are always `>= 1`, so such a predicate matches no
rows and the correct result is empty — it must not panic. The
`WindowTopN` rule's `extract_window_limit` maps `rn < K` to a fetch of
`K - 1`; for `K = 1` that fetch is `0`, which is passed to
`PartitionedTopKExec::try_new`, whose `assert!(k > 0)` panics. With the
optimization disabled the same query correctly returns no rows.
## What changes are included in this PR?
- In `WindowTopN::try_transform`, bail out of the rewrite when the
computed limit is `0`, letting the regular `FilterExec` produce the
(empty) result.
- Add regression cases to `window_topn.slt` for `rn < 1`, `1 > rn`, and
`rn <= 0` (all must return empty without panicking).
## Are these changes tested?
Yes — the new sqllogictest cases fail (panic) without the fix and pass
with it. Existing `window_topn` tests continue to pass.
## Are there any user-facing changes?
No API changes. `rn < 1` / `1 > rn` now returns an empty result instead
of panicking when `enable_window_topn` is on, matching the behavior when
it is off.
---
datafusion/physical-optimizer/src/window_topn.rs | 9 +++++++++
datafusion/sqllogictest/test_files/window_topn.slt | 23 ++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/datafusion/physical-optimizer/src/window_topn.rs b/datafusion/physical-optimizer/src/window_topn.rs
index 20bd8b0d38..aa46a5a9a7 100644
--- a/datafusion/physical-optimizer/src/window_topn.rs
+++ b/datafusion/physical-optimizer/src/window_topn.rs
@@ -141,6 +141,15 @@ impl WindowTopN {
// Step 2: Extract limit from predicate (rn <= K, rn < K, etc.)
let (col_idx, limit_n) = extract_window_limit(filter.predicate())?;
+ // A predicate such as `rn < 1` (or the flipped `1 > rn`) yields a fetch of
+ // 0. `ROW_NUMBER`/`RANK` are always >= 1, so no row can satisfy it and the
+ // correct result is empty. `PartitionedTopKExec` requires `k > 0` and would
+ // panic on `k = 0`, so bail out here and let the regular `FilterExec` produce
+ // the (empty) result instead of rewriting.
+ if limit_n == 0 {
+ return None;
+ }
+
// Step 3: Walk through optional ProjectionExec and RepartitionExec to find BoundedWindowAggExec
let child = filter.input();
let (window_exec, intermediates) = find_window_below(child)?;
diff --git a/datafusion/sqllogictest/test_files/window_topn.slt b/datafusion/sqllogictest/test_files/window_topn.slt
index 44cb31153b..a9a52a654f 100644
--- a/datafusion/sqllogictest/test_files/window_topn.slt
+++ b/datafusion/sqllogictest/test_files/window_topn.slt
@@ -84,6 +84,29 @@ SELECT id, pk, val FROM (
8 3 100
9 3 50
+# Test 3b: rn < 1 has fetch = 0. ROW_NUMBER is always >= 1 so no row qualifies and
+# the result must be empty. The rewrite must not build a PartitionedTopKExec with k = 0
+# (which panics); it falls back to the regular filter. Regression for the k = 0 panic.
+query III rowsort
+SELECT id, pk, val FROM (
+ SELECT *, ROW_NUMBER() OVER (PARTITION BY pk ORDER BY val) as rn FROM window_topn_t
+) WHERE rn < 1;
+----
+
+# Test 3c: flipped form `1 > rn` (fetch = 0) — same empty result, no panic.
+query III rowsort
+SELECT id, pk, val FROM (
+ SELECT *, ROW_NUMBER() OVER (PARTITION BY pk ORDER BY val) as rn FROM window_topn_t
+) WHERE 1 > rn;
+----
+
+# Test 3d: rn <= 0 (fetch = 0) — empty, no panic.
+query III rowsort
+SELECT id, pk, val FROM (
+ SELECT *, ROW_NUMBER() OVER (PARTITION BY pk ORDER BY val) as rn FROM window_topn_t
+) WHERE rn <= 0;
+----
+
# Test 4: Without PARTITION BY — should NOT optimize
query II rowsort
SELECT id, val FROM (
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org