Thread: Cleanup: Duplicated, misplaced comment in HeapScanDescData

Cleanup: Duplicated, misplaced comment in HeapScanDescData

From
Matthias van de Meent
Date:
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

Attachment

Re: Cleanup: Duplicated, misplaced comment in HeapScanDescData

From
Matthias van de Meent
Date:
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

Attachment

Re: Cleanup: Duplicated, misplaced comment in HeapScanDescData

From
Andres Freund
Date:
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