ump45nose opened a new pull request, #26432: URL: https://github.com/apache/datafusion/pull/26432
## Which issue does this PR close? - Closes #26431. ## Rationale for this change `ORDER BY ... LIMIT 0` panics with `assertion failed: k > 0` when the plan reaches `SortExec: TopK(fetch=0)`. The default optimizer hides this because `eliminate_limit` replaces `LIMIT 0` with an empty relation, but it is reachable with `datafusion.optimizer.max_passes = 0`, with custom optimizer rules, or by building `SortExec::with_fetch(Some(0))` directly. A `LIMIT 0` query should return no rows, not panic. ## What changes are included in this PR? - `TopKHeap::new` no longer asserts `k > 0`. - `TopK::try_new` marks a `k = 0` TopK as `finished` up front, and `TopK::insert_batch` returns early when `k = 0` (no row can enter the heap). - `SortExec` checks `topk.finished` before polling the next input batch, so a `fetch = 0` TopK does not read its input. The loop behaves the same as before for `k > 0`. The fix is in `TopK` itself, not a separate empty-stream branch in `SortExec`. This way `emit()` still runs and marks the shared TopK dynamic filter as emitted/complete, as it does for any other TopK. ## What is the testing strategy for this PR? - New unit test `topk::tests::test_topk_with_zero_k`: a `k = 0` TopK is finished at construction, ignores an inserted batch, emits no rows, and marks its dynamic filter complete. Without this change it panics in `TopKHeap::new`. - New `topk.slt` case that sets `datafusion.optimizer.max_passes = 0`, checks that the plan is `SortExec: TopK(fetch=0)`, and checks that `select * from topk order by x limit 0` returns no rows. - Locally: `cargo test -p datafusion-physical-plan --lib -- topk sorts::`, the `topk.slt`, `limit*.slt` and `order.slt` sqllogictests, `cargo fmt --all -- --check` and `cargo clippy -p datafusion-physical-plan --all-targets --all-features -- -D warnings` all pass. The new `topk.slt` case panics with `assertion failed: k > 0` without the fix. ## Are there any user-facing changes? No API changes. `LIMIT 0` with a TopK sort now returns an empty result instead of panicking. --- This PR was prepared with AI assistance. I reviewed the change and ran the tests listed above. -- 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]
