| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
| Cc: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*) |
| Date: | 2025-11-05 00:16:58 |
| Message-ID: | CAApHDvr57aOVMVT0LrS4JE3QsVHnGDA2=m=3824A5XCTSS-ukw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 5 Nov 2025 at 08:51, Matheus Alcantara <matheusssilv97(at)gmail(dot)com> wrote:
>
> On Mon Nov 3, 2025 at 7:47 PM -03, David Rowley wrote:
> > 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.
Did you look at: git diff origin/master..master ?
I've certainly accidentally periodically committed to my local master
which I ended up doing: git reset --hard origin/master to fix
> 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 for the review. I might not have been clear that I had only
intended the NOT_USED part as an example for during the review period.
I'd never intended it going any further.
I've attached a version with the NOT_USED part removed (and a bunch of
#includes I forgot to remove). The only other change was a minor
revision to some comments.
The primary concern I have now is when in planning that we do this
Aggref simplification. Maybe I shouldn't be too concerned about that
as there doesn't seem to be a current reason not to put it where it
is. If someone comes up with a reason to do it later in planning at
some point in the future, we can consider moving it then. That sort of
excludes extensions with aggregates that want to have a
SupportRequestSimplifyAggref support function that might need the
processing done later in planning, but that just feels like a
situation that's unlikely to arise.
David
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Have-the-planner-replace-COUNT-ANY-with-COUNT-whe.patch | application/octet-stream | 22.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joel Jacobson | 2025-11-05 00:58:23 | Re: Optimize LISTEN/NOTIFY |
| Previous Message | Jacob Champion | 2025-11-05 00:14:52 | Re: [BUG] PostgreSQL crashes with ThreadSanitizer during early initialization |