Thread: Re: index_delete_sort: Unnecessary variable "low" is used in heapam.c
> On 24 Sep 2024, at 10:32, btnakamurakoukil <btnakamurakoukil@oss.nttdata.com> wrote: > I noticed unnecessary variable "low" in index_delete_sort() (/postgres/src/backend/access/heap/heapam.c), patch attached.What do you think? That variable does indeed seem to not be used, and hasn't been used since it was committed in d168b666823. The question is if it's a left-over from development which can be removed, or if it should be set and we're missing an optimization. Having not read the referenced paper I can't tell so adding Peter Geoghegan who wrote this for clarification. -- Daniel Gustafsson
On 2024/09/24 21:31, Daniel Gustafsson wrote: >> On 24 Sep 2024, at 10:32, btnakamurakoukil <btnakamurakoukil@oss.nttdata.com> wrote: > >> I noticed unnecessary variable "low" in index_delete_sort() (/postgres/src/backend/access/heap/heapam.c), patch attached.What do you think? > > That variable does indeed seem to not be used, and hasn't been used since it > was committed in d168b666823. The question is if it's a left-over from > development which can be removed, or if it should be set and we're missing an > optimization. Having not read the referenced paper I can't tell so adding > Peter Geoghegan who wrote this for clarification. It's been about a month without updates. How about removing the unnecessary variable as suggested? We can always add the "missing" logic later if needed. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
> On 5 Nov 2024, at 17:40, Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > On 2024/09/24 21:31, Daniel Gustafsson wrote: >>> On 24 Sep 2024, at 10:32, btnakamurakoukil <btnakamurakoukil@oss.nttdata.com> wrote: >>> I noticed unnecessary variable "low" in index_delete_sort() (/postgres/src/backend/access/heap/heapam.c), patch attached.What do you think? >> That variable does indeed seem to not be used, and hasn't been used since it >> was committed in d168b666823. The question is if it's a left-over from >> development which can be removed, or if it should be set and we're missing an >> optimization. Having not read the referenced paper I can't tell so adding >> Peter Geoghegan who wrote this for clarification. > > It's been about a month without updates. How about removing the unnecessary > variable as suggested? We can always add the "missing" logic later if needed. Thanks for reviving this, I had admittedly forgotten about this thread. I don't see any reason to not go ahead with the proposed diff. -- Daniel Gustafsson
> On 5 Nov 2024, at 22:08, Daniel Gustafsson <daniel@yesql.se> wrote: > >> On 5 Nov 2024, at 17:40, Fujii Masao <masao.fujii@oss.nttdata.com> wrote: >> >> On 2024/09/24 21:31, Daniel Gustafsson wrote: >>>> On 24 Sep 2024, at 10:32, btnakamurakoukil <btnakamurakoukil@oss.nttdata.com> wrote: >>>> I noticed unnecessary variable "low" in index_delete_sort() (/postgres/src/backend/access/heap/heapam.c), patch attached.What do you think? >>> That variable does indeed seem to not be used, and hasn't been used since it >>> was committed in d168b666823. The question is if it's a left-over from >>> development which can be removed, or if it should be set and we're missing an >>> optimization. Having not read the referenced paper I can't tell so adding >>> Peter Geoghegan who wrote this for clarification. >> >> It's been about a month without updates. How about removing the unnecessary >> variable as suggested? We can always add the "missing" logic later if needed. > > Thanks for reviving this, I had admittedly forgotten about this thread. I > don't see any reason to not go ahead with the proposed diff. Done now. -- Daniel Gustafsson