It may be possible to apply transformations to other aggregate functions too, but I don't want to discuss that here. I mostly want to determine if the infrastructure is ok and do the count(*) one because it seems like the most likely one to be useful.
One thing I wasn't too sure about was if we should make it possible for the support function to return something that's not an Aggref. In theory, something like COUNT(NULL) could just return '0'::bigint. While that does seem an optimisation that wouldn't be applied very often, I have opted to leave it so that such an optimisation *could* be done by the support function. I also happen to test that that doesn't entirely break the query, as ordinarily it would if we didn't have Query.hasAggs (It's not too dissimilar to removing unused columns from a subquery)
Should we do this?
David
+1
Automatic query improvements are a Good Thing. We're never going to educate everyone that COUNT(1) is an anti-pattern, so it's easier to make it not an anti-pattern.
I'm in favor of the COUNT(NULL) optimization as well, as one of my favorite programming tropes is "There is nothing faster than nothing".
The check seems lightweight enough to me. Applies clean and tests pass. Test coverage seems to cover all the cases.