Kristofferjoha commented on issue #27130: URL: https://github.com/apache/datafusion/issues/27130#issuecomment-6665273783
@alamb Sorry, my bad. #26670 assumed there's only ever one reset copy running, which is true for recursive CTEs but not for cached plans. +1 on the revert for 56. FYI though, AggregateExec has the same problem on main. I tested `SELECT max(x) FROM t WHERE x % 2 = 0` on a parquet file (x = 1..1000, 10 row groups), reset the plan twice and ran both copies. The first returns 1000, the second returns NULL because its scan picks up the first copy's `x > 1000` filter. Running them concurrently is worse, most of the time you get wrong but plausible values like 900 or 800. It's not new in 56 though. On 55.2.0 AggregateExec didn't override reset_state at all, so even reset -> run -> reset -> run gave NULL (#26054). #26056 fixed that part, so I don't think it should be reverted along with #26670. | | 55.2.0 | main | |---|---|---| | topk limit 1, reset x2 then run both | ok | 2nd: no rows | | max, reset/run/reset/run | 2nd: NULL | ok | | max, reset x2 then run both | 2nd: NULL | 2nd: NULL | | max, 2 copies concurrently (50 runs) | 40 wrong | 45 wrong | Haven't tested HashJoin, but since its filter just describes the build side I'd guess it's fine. @raghavyadav01 I think option 1 is the real fix as you mention, since it would cover both topk and aggregate and keep the CTE pruning. -- 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]
