neilconway commented on PR #25849: URL: https://github.com/apache/datafusion/pull/25849#issuecomment-5914339814
@wudidapaopao Thanks for this! I think this is a very useful optimization for some common query shapes. If I understand correctly, there are a few distinct optimizations here: 1. Rewrite `count(non-nullable-col)` -> `count(1)`, based on nullability. This seems very useful but I think it would make sense to split it off into a separate PR. 2. Rewrite `count(*) / count(1)` -> `count()`. I agree that there is wasted work in the current evaluation of `count(1)`, but instead of rewriting `count(1)` to a nullary aggregate (and adding support for nullary aggregates), what if we just optimized the evaluation of `count(1)` and other aggregates passed constant inputs? This would also benefit other situations, like the common case where `string_agg` is called with a constant separator, or `nth_value(col, const-n)`. I suspect it would also be simpler than the current PR: no API changes, `EXPLAIN` churn, unparser handling, etc. I had Claude Code prototype a [quick implementation of caching for const agg args](https://gist.github.com/neilconway/7d634396ffca48e3bf81cdfcbfe5f61e) -- based on some quick benchmarks, it matches the performance of this PR for the `count(1)` case, and also is a small win for other cases like `nth_value(col, const-n)`. -- 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]
