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

Hello,

Le jeu. 5 sept. 2024 à 08:19, Guillaume Lelarge <guillaume@lelarge.info> a
écrit :

> Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot <
> bertranddrouvot.pg@gmail.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.pg@gmail.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 :)
>
>

Finally found some time to work on this. Tests added on v5 patch (attached).

Regards.


-- 
Guillaume.