Re: pg_stat_statements: more test coverage

On 29.12.23 06:14, Julien Rouhaud wrote:

> 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.

Yeah, I think I added that query before I added the queries to check the
contents of pg_stat_statements.query itself, so it's a bit obsolete. I
reworked that part.

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

I added that.

>
>> 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.

Ok, I have committed these two patches.

>> 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.

Ok, skipped that.