Re: Comment typo in tableam.h - Mailing list pgsql-hackers

From Ashwin Agrawal
Subject Re: Comment typo in tableam.h
Date
Msg-id CALfoeisPsn1tQ67yZyGHSrO_XYw3Ttn9iZt6NrW_yNGYxaWRQA@mail.gmail.com
Whole thread Raw
In response to Re: Comment typo in tableam.h  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers

On Mon, Jun 3, 2019 at 6:24 PM Andres Freund <andres@anarazel.de> wrote:
Hi,

On 2019-06-03 18:21:56 -0700, Ashwin Agrawal wrote:
> On Mon, Jun 3, 2019 at 5:26 PM Andres Freund <andres@anarazel.de> wrote:
>
> > Hi,
> >
> > Thanks for these!
> >
> > On 2019-06-03 17:24:15 -0700, Ashwin Agrawal wrote:
> > >       /*
> > >        * Estimate the size of shared memory needed for a parallel scan
> > of this
> > > -      * relation. The snapshot does not need to be accounted for.
> > > +      * relation.
> > >        */
> > >       Size            (*parallelscan_estimate) (Relation rel);
> >
> > That's not a typo?
> >
>
> The snapshot is not passed as argument to that function hence seems weird
> to refer to snapshot in the comment, as anyways callback function can't
> account for it.

It's part of the parallel scan struct, and it used to be accounted for
by pre tableam function...

Reads like the comment written from past context then, and doesn't have much value now. Its confusing than helping, to state not to account for snapshot and not any other field.
table_parallelscan_estimate() has snapshot argument and it accounts for it, but callback doesn't. I am not sure how a callback would explicitly use that comment and avoid accounting for snapshot if its using generic ParallelTableScanDescData. But if you feel is helpful, please feel free to keep that text.

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Print baserestrictinfo for varchar fields
Next
From: Michael Paquier
Date:
Subject: Re: coverage additions