Re: Comment typo in tableam.h

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.