lxc512157407 commented on PR #24821: URL: https://github.com/apache/datafusion/pull/24821#issuecomment-5665386039
Thanks @2010YOUY01 — your earlier feedback about not baking special cases into execution paths was the catalyst for the redesign. The new approach (dc92a7f06) is fundamentally different from what you reviewed: Before (rejected): Build-time simplification in FilterExecBuilder. The predicate became lit(true), FilterPushdown then deleted the FilterExec node, plan shape changed, and joins regressed 45% because the implicit batch coalescing was lost. After (current): No plan change at all. The FilterExec node stays exactly where main puts it. The fast path is purely a per-batch runtime micro-optimization inside FilterExecStream::poll_next: when the incoming batch's column reports null_count == 0 (an O(1) cached value), skip mask evaluation and filter_record_batch for that batch only. A batch that contains NULLs takes the normal path. Plan shape, repartition decisions, and batch coalescing are all identical to main (verified via EXPLAIN ANALYZE). So this is not statistics-based expression optimization anymore — it is a data-dependent short circuit, closer in spirit to how operators already skip work when they observe the data allows it (e.g. all-true boolean arrays, zero-row batches). The statistics-driven simplification idea you and I discussed belongs in the logical layer and I agree it should be a separate effort if pursued. Would appreciate your take on the current shape. -- 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]
