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: [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]
