jayzhan211 commented on PR #26436: URL: https://github.com/apache/datafusion/pull/26436#issuecomment-6290293796
> One optional idea: arrow only keeps the source buffers on its sparse path (at most 1/16 selected), so the fallback could also check `filter.true_count() * 16 <= filter.len()` and keep the fused path for dense filters. Thanks for the review! I looked into this, and I don't think the extra check would change what runs for dense filters. When more than 1/16 of the rows are selected, arrow's `push_batch_with_filter` already filters the batch and then calls `push_batch` ([arrow-select 60.0.0, coalesce.rs L652-L665](https://github.com/apache/arrow-rs/blob/60.0.0/arrow-select/src/coalesce.rs#L652-L665)). That's the same thing the fallback here does, so `WHERE "SearchPhrase" <> ''` runs the same code with or without the check. I think the Q13 and Q14 numbers are runner noise. Q4, Q5 and Q18 have no `WHERE` clause, so they never reach this code, but they moved by about the same amount (7-11% slower) in the same run. On my machine, Q13 was 440-478 ms on main and 435-469 ms with this PR across three interleaved runs. I'd also rather not copy arrow's private 1/16 threshold here. If arrow changes it, the two checks could drift apart and the leak could come back for filters in between. Since this fallback goes away once apache/arrow-rs#11635 is fixed, I'd keep it as is. Happy to add the check if you still prefer it, though. -- 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]
