raghavyadav01 opened a new pull request, #27142: URL: https://github.com/apache/datafusion/pull/27142
## Which issue does this PR close? - Closes #27130. ## Rationale for this change As discussed in #27130, this reverts #26670 so copies made by `SortExec::reset_state()` no longer share one TopK dynamic filter. When a cached `LIMIT 1` plan was reset twice and both copies ran, the first one to finish tightened the shared filter and the second returned no rows. A plain revert brings back a different wrong result, though: a plan that has run once and is then reset still has its old bound in the scan's predicate (the scan keeps the filter it got at planning time). For `SELECT a FROM t ORDER BY a LIMIT 1` over a parquet file with pushdown enabled, the rerun returned no rows with the plain revert. So this keeps the in-place `reset()` from #26056 on the old filter and then gives the new plan its own filter, as @Kristofferjoha suggested keeping. | | main | plain revert | this PR | |---|---|---|---| | reset twice, run both copies (#27130) | 2nd: no rows | ok | ok | | run, reset, run again (parquet, pushdown) | ok | 2nd: no rows | ok | ## What changes are included in this PR? - `SortExec::reset_state` resets the old dynamic filter in place, then creates a new one for the reset plan. The `sort_concurrency` reset from #26242 is kept. (The plain revert did not apply cleanly because #26242 changed the same lines.) - Removes the recursive CTE pruning test #26670 added to `cte.slt`, since the scan now keeps the old filter (reset to `true`) and the pruning no longer happens, as in 55. - Adds two unit tests in `sorts/sort.rs`: the reproducer from #27130, and a test that the filter a scan would hold is back to `true` after `reset_state`. The second one fails with the plain revert. ## Are these changes tested? Yes. Both new tests fail on main or on the plain revert respectively, and pass here. All `datafusion-physical-plan` unit tests and sqllogictests pass. ## Are there any user-facing changes? The TopK dynamic filter inside a recursive CTE no longer prunes after the first iteration, as in 55 (the #26670 test's scan reads 300 rows instead of 30). Query results are unchanged. The proper fix (a fresh filter per copy that the scan also picks up, which would also cover `AggregateExec`) can follow after 56. This reopens #26642. -- 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]
