index_delete_sort: Unnecessary variable "low" is used in heapam.c
Hi hackers,
I noticed unnecessary variable "low" in index_delete_sort()
(/postgres/src/backend/access/heap/heapam.c), patch attached. What do
you think?
Regards,
Koki Nakamura <btnakamurakoukil@oss.nttdata.com>
Attachments:
0001-Delete-low.patchtext/x-diff; name=0001-Delete-low.patchDownload+1-3
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