Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes

Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
a écrit :

> Hi,
>
> On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote:
> > Hi,
> >
> > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <
> bertranddrouvot(dot)pg(at)gmail(dot)com>
> > a écrit :
> > > What about adding a comment instead of this extra check?
> > >
> > >
> > Done too in v3.
>
> Thanks!
>
> 1 ===
>
> + /*
> + * Don't check counts.parallelnumscans because counts.numscans
> includes
> + * counts.parallelnumscans
> + */
>
> "." is missing at the end of the comment.
>
>
Fixed in v4.

> 2 ===
>
> - if (t > tabentry->lastscan)
> + if (t > tabentry->lastscan && lstats->counts.numscans)
>
> The extra check on lstats->counts.numscans is not needed as it's already
> done
> a few lines before.
>
>
Fixed in v4.

> 3 ===
>
> + if (t > tabentry->parallellastscan &&
> lstats->counts.parallelnumscans)
>
> This one makes sense.
>
> And now I'm wondering if the extra comment added in v3 is really worth it
> (and
> does not sound confusing)? I mean, the parallel check is done once we passe
> the initial test on counts.numscans. I think the code is clear enough
> without
> this extra comment, thoughts?
>
>
I'm not sure I understand you here. I kinda like the extra comment though.

> 4 ===
>
> What about adding a few tests? or do you want to wait a bit more to see if
> "
> there's an agreement on this patch" (as you stated at the start of this
> thread).
>
>
Guess I can start working on that now. It will take some time as I've never
done it before. Good thing I added the patch on the November commit fest :)

Thanks again.

Regards.

--
Guillaume.