Thread

Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*)

Matheus Alcantara <matheusssilv97@gmail.com> — 2025-11-04T19:51:47Z

On Mon Nov 3, 2025 at 7:47 PM -03, David Rowley wrote:
> On Tue, 4 Nov 2025 at 09:38, Matheus Alcantara <matheusssilv97@gmail.com> wrote:
>> I looked the code and it seems to be in a good shape, but I tried to
>> apply the v2 on top of e7ccb247b38 in master to run some tests and a
>> rebase is necessary.
>
> Are you sure you've not got something else in your branch? It applies
> ok here, and the CFbot isn't complaining either. CFBot's is based on
> cf8be0225, which is 2 commits before the one you're trying, but
> src/test/regress/expected/aggregates.out hasn't been changed since
> 2025-10-07.
>
Yes, my branch is clean, I even tried to apply on a cleaned git clone
but it is still failling to apply, very strange. I've added the cfbot
remote and cherry picked your commit and this works. I'll investigate
later why I'm not able to apply your patch directly.

I've tested and benchmarked the patch using count(1) and
count(not_null_col) and I've got similar results, ~30% of improvements
compared with master.

The code seems good to me, I don't have too many comments, I'm just not
sure if we should keep the #ifdef NOT_USED block but I'm not totally
against it. I'm +1 for the idea.

Thanks

--
Matheus Alcantara
EDB: http://www.enterprisedb.com