You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alpaylan (via GitHub)" <gi...@apache.org> on 2026/10/16 20:42:20 UTC

[I] `ORDER BY ... LIMIT 0` panics in TopK (`assertion failed: k > 0`) when `eliminate_limit` does not run [datafusion]

alpaylan opened a new issue, #26431:
URL: https://github.com/apache/datafusion/issues/26431

   ### Describe the bug
   
   A `SortExec` with `fetch = Some(0)` over unsorted input builds a `TopK` with `k = 0`, and `TopKHeap::new` panics on `assert!(k > 0)` instead of returning an empty result.
   
   With the default configuration, `eliminate_limit` turns `LIMIT 0` into an empty relation first, so this is not reached. But it is reachable from plain SQL by setting `datafusion.optimizer.max_passes = 0`. The physical limit pushdown then puts `fetch=0` on the sort. Embedders that remove or replace the logical optimizer rules, or build `SortExec::with_fetch(Some(0))` directly, hit it too.
   
   ### To Reproduce
   
   
   `datafusion-cli` built from `main` (63a2801c1b0b766cc206c2bbebde9c52dac66f60):
   
   ```sql
   SET datafusion.optimizer.max_passes = 0;
   CREATE TABLE t0 (c0 BIGINT) AS VALUES (3), (1), (2);
   SELECT c0 FROM t0 ORDER BY c0 LIMIT 0;
   ```
   
   ```
   thread 'main' panicked at datafusion/physical-plan/src/topk/mod.rs:946:9:
   assertion failed: k > 0
   ```
   
   `EXPLAIN` shows the plan is `SortExec: TopK(fetch=0)` over the scan:
   
   ```sql
   > EXPLAIN SELECT c0 FROM t0 ORDER BY c0 LIMIT 0;
   +---------------+-------------------------------+
   | plan_type     | plan                          |
   +---------------+-------------------------------+
   | physical_plan | ┌───────────────────────────┐ |
   |               | │       SortExec(TopK)      │ |
   |               | │    --------------------   │ |
   |               | │    c0@0 ASC NULLS LAST    │ |
   |               | │                           │ |
   |               | │          limit: 0         │ |
   |               | └─────────────┬─────────────┘ |
   |               | ┌─────────────┴─────────────┐ |
   |               | │       DataSourceExec      │ |
   |               | │    --------------------   │ |
   |               | │         bytes: 128        │ |
   |               | │       format: memory      │ |
   |               | │          rows: 3          │ |
   |               | └───────────────────────────┘ |
   |               |                               |
   +---------------+-------------------------------+
   ```
   
   ### Expected behavior
   
   An empty result (0 rows), as without the `SET`. `LIMIT 0` is valid, and the SQL reference says the count must be a
   non-negative integer.
   
   ### Additional context
   
   In `SortExec::execute` (`datafusion/physical-plan/src/sorts/sort.rs`), the `(false, Some(fetch))` arm passes `fetch`
   to `TopK::try_new` without checking for 0. The `(true, Some(0))` arm is fine, because it uses `LimitStream`.
   
   Code references (at 63a2801c1):
   
   - The panic: `TopKHeap::new` asserts `k > 0`,
     [`topk/mod.rs#L944-L946`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-plan/src/topk/mod.rs#L944-L946).
     It is called with the fetch as `k` from `TopK::try_new`,
     [`topk/mod.rs#L408`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-plan/src/topk/mod.rs#L408).
   - The missing check: the `match (sort_satisfied, self.fetch)` in `SortExec::execute`,
     [`sorts/sort.rs#L1609-L1629`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-plan/src/sorts/sort.rs#L1609-L1629).
     `(true, Some(fetch))` uses `LimitStream` and handles 0. `(false, Some(fetch))`
     ([L1617-L1629](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-plan/src/sorts/sort.rs#L1617-L1629))
     goes straight to `TopK::try_new`.
   - Why default settings don't hit it: `EliminateLimit` rewrites a literal `LIMIT 0` to an `EmptyRelation`,
     [`eliminate_limit.rs#L64-L72`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/optimizer/src/eliminate_limit.rs#L64-L72).
   - Why `max_passes = 0` hits it: the logical optimizer loop runs `while i < max_passes`,
     [`optimizer.rs#L621`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/optimizer/src/optimizer.rs#L621),
     so no logical rule runs
     ([config doc, `config.rs#L1841-L1844`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/common/src/config.rs#L1841-L1844)).
     The physical `LimitPushdown` then folds the limit into the sort through `with_fetch`,
     [`limit_pushdown.rs#L276`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-optimizer/src/limit_pushdown.rs#L276),
     which gives `SortExec: TopK(fetch=0)`.
   - The dynamic filter a fix must complete: `mark_topk_emitted`,
     [`topk/mod.rs#L327`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/datafusion/physical-plan/src/topk/mod.rs#L327).
   - `LIMIT 0` is valid SQL:
     [`select.md#L900`](https://github.com/apache/datafusion/blob/63a2801c1b0b766cc206c2bbebde9c52dac66f60/docs/source/user-guide/sql/select.md?plain=1#L900).
   
   This is the same failure as #24404 (`PartitionedTopK requires k > 0`), which #24405 fixed by skipping the WindowTopN rewrite when the fetch is 0. This one is in the plain `TopK` path.
   
   A possible fix is a `(false, Some(0))` arm that returns an empty stream without polling the input. It must also mark
   the TopK dynamic filter as complete, so a scan waiting on it does not hang. I have a patch with a unit test and can
   open a PR.
   
   (Found with an automated differential-testing tool; I reproduced it by hand on current `main`.)
   
   ### Willingness to contribute
   
   I would like to contribute a fix for this bug, but request guidance from the DataFusion community


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] `ORDER BY ... LIMIT 0` panics in TopK (`assertion failed: k > 0`) when `eliminate_limit` does not run [datafusion]

Posted by "jayzhan211 (via GitHub) via github" <gi...@datafusion.apache.org>.
jayzhan211 closed issue #26431: `ORDER BY ... LIMIT 0` panics in TopK (`assertion failed: k > 0`) when `eliminate_limit` does not run
URL: https://github.com/apache/datafusion/issues/26431


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] `ORDER BY ... LIMIT 0` panics in TopK (`assertion failed: k > 0`) when `eliminate_limit` does not run [datafusion]

Posted by "shahwazreza (via GitHub) via github" <gi...@datafusion.apache.org>.
shahwazreza commented on issue #26431:
URL: https://github.com/apache/datafusion/issues/26431#issuecomment-6536482963

   I'd like to work on this.
   
   The panic comes from the `(false, Some(fetch))` arm in `SortExec::execute`, which passes `fetch = 0` straight into `TopK::try_new`, and `TopKHeap::new` asserts `k > 0`. Removing the assert wouldn't be enough, because `TopKHeap::add` also assumes `k > 0` (it calls `peek_mut().unwrap()` when `len == k`).
   
   My plan is to handle it in `SortExec` instead: add a `(false, Some(0))` arm that returns an empty stream. One thing to be careful about is the shared TopK dynamic filter. It only completes after every partition calls `mark_topk_emitted()`, so the new arm still has to call it. Otherwise anything waiting on `wait_complete()` would hang.
   
   For tests I have:
   - a unit test in `sort.rs` with two partitions and `fetch = Some(0)` that checks both return no rows and that the filter completes only after the second one
   - a sqllogictest in `limit.slt` with the repro from this issue (`max_passes = 0`), plus an `EXPLAIN` so it still covers `TopK(fetch=0)`
   
   I have it working locally and will open a PR shortly.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org