Re: pg_stat_statements: more test coverage

On Wed, Dec 27, 2023 at 8:53 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 27.12.23 09:08, Julien Rouhaud wrote:
> >
> > I was a bit surprised by that so I checked locally.  It does work as
> > expected provided that you set pg_stat_statements.track to all:
>
> Ok, here is an updated patch set that does it that way.

It looks good to me.  One minor complaint, I'm a bit dubious about
those queries:

SELECT count(*) <= 100 AND count(*) > 0 FROM pg_stat_statements;

Is it to actually test that pg_stat_statements won't store more than
pg_stat_statements.max records?  Note also that this query can't
return 0 rows, as the currently executed query will have an entry
added during post_parse_analyze.  Maybe a comment saying what this is
actually testing would help.

It would also be good to test that pg_stat_statements_info.dealloc is
more than 0 once enough statements have been issued.

> I have committed the patches 0002 and 0003 from the previous patch set
> already.
>
> I have also enhanced the TAP test a bit to check the actual content of
> the output across restarts.

Nothing much to say about this one, it all looks good.

> I'm not too hung up on the 0001 patch if others don't like that approach.

I agree with Michael on this one, the only times I saw this pattern
was to comply with some company internal policy for minimal coverage
numbers.