Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*)
| От | David Rowley |
|---|---|
| Тема | Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*) |
| Дата | |
| Msg-id | CAApHDvr57aOVMVT0LrS4JE3QsVHnGDA2=m=3824A5XCTSS-ukw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*) ("Matheus Alcantara" <matheusssilv97@gmail.com>) |
| Ответы |
Re: Have the planner convert COUNT(1) / COUNT(not_null_col) to COUNT(*)
|
| Список | pgsql-hackers |
On Wed, 5 Nov 2025 at 08:51, Matheus Alcantara <matheusssilv97@gmail.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
Вложения
В списке pgsql-hackers по дате отправления: