Cleanup: Duplicated, misplaced comment in HeapScanDescData
Hi,
I noticed that the comment on/beneath rs_numblocks in HeapScanDescData
is duplicated above rs_strategy. I don't know if there should have
been a different comment above rs_strategy, but the current one is
definitely out of place, so I propose to remove it as per attached.
The comment was duplicated in c2fe139c20 with the update to the table
scan APIs, which was first seen in PostgreSQL 11.
Kind regards,
Matthias van de Meent
Attachments:
v1-0001-Remove-duplicate-of-comment-under-rs_numblocks.patchapplication/octet-stream; name=v1-0001-Remove-duplicate-of-comment-under-rs_numblocks.patchDownload
From 5f3bdaf3c498cabd8fd32e29a83f4ade23f1d04f Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 21 Nov 2022 11:59:04 +0100
Subject: [PATCH v1] Remove duplicate of comment under rs_numblocks
The comment was duplicated with the refactoring of the table scan APIs
in c2fe139c20 for PostgreSQL 11.
---
src/include/access/heapam.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index fd67d500dc..a087aaa2f7 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -60,7 +60,6 @@ typedef struct HeapScanDescData
Buffer rs_cbuf; /* current buffer in scan, if any */
/* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */
- /* rs_numblocks is usually InvalidBlockNumber, meaning "scan whole rel" */
BufferAccessStrategy rs_strategy; /* access strategy for reads */
HeapTupleData rs_ctup; /* current tuple in scan, if any */
--
2.30.2
On Mon, 21 Nov 2022 at 12:12, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
Hi,
I noticed that the comment on/beneath rs_numblocks in HeapScanDescData
is duplicated above rs_strategy. I don't know if there should have
been a different comment above rs_strategy, but the current one is
definitely out of place, so I propose to remove it as per attached.The comment was duplicated in c2fe139c20 with the update to the table
scan APIs, which was first seen in PostgreSQL 11.
I made a mistake in determining this version number; it was PostgreSQL
12 where this commit was first included. Attached is the same patch
with the description updated accordingly.
Kind regards,
Matthias van de Meent
Attachments:
v2-0001-Remove-duplicate-of-comment-under-rs_numblocks.patchapplication/octet-stream; name=v2-0001-Remove-duplicate-of-comment-under-rs_numblocks.patchDownload
From 5f3bdaf3c498cabd8fd32e29a83f4ade23f1d04f Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 21 Nov 2022 11:59:04 +0100
Subject: [PATCH v2] Remove duplicate of comment under rs_numblocks
The comment was duplicated with the refactoring of the TableAM APIs
in c2fe139c20 for PostgreSQL 12.
---
src/include/access/heapam.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index fd67d500dc..a087aaa2f7 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -60,7 +60,6 @@ typedef struct HeapScanDescData
Buffer rs_cbuf; /* current buffer in scan, if any */
/* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */
- /* rs_numblocks is usually InvalidBlockNumber, meaning "scan whole rel" */
BufferAccessStrategy rs_strategy; /* access strategy for reads */
HeapTupleData rs_ctup; /* current tuple in scan, if any */
--
2.30.2
Hi,
On 2022-11-21 12:34:12 +0100, Matthias van de Meent wrote:
On Mon, 21 Nov 2022 at 12:12, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:Hi,
I noticed that the comment on/beneath rs_numblocks in HeapScanDescData
is duplicated above rs_strategy. I don't know if there should have
been a different comment above rs_strategy, but the current one is
definitely out of place, so I propose to remove it as per attached.The comment was duplicated in c2fe139c20 with the update to the table
scan APIs, which was first seen in PostgreSQL 11.I made a mistake in determining this version number; it was PostgreSQL
12 where this commit was first included. Attached is the same patch
with the description updated accordingly.
I guess that happened because of the odd placement of the comment from
before the change:
bool rs_temp_snap; /* unregister snapshot at scan end? */
-
- /* state set up at initscan time */
- BlockNumber rs_nblocks; /* total number of blocks in rel */
- BlockNumber rs_startblock; /* block # to start at */
- BlockNumber rs_numblocks; /* max number of blocks to scan */
- /* rs_numblocks is usually InvalidBlockNumber, meaning "scan whole rel" */
- BufferAccessStrategy rs_strategy; /* access strategy for reads */
bool rs_syncscan; /* report location to syncscan logic? */
We rarely put comments document a struct member after it.
I'm inclined to additionally move the "legitimate" copy of the comment
to before rs_numblocks, rather than after it.
Greetings,
Andres Freund