Re: pg_stat_statements: more test coverage

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Julien Rouhaud <rjuju123(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_stat_statements: more test coverage
Date: 2023-12-30 19:39:47
Message-ID: 3c447364-c446-4341-8478-092ab1995408@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2023-12-30 21:51:34 Re: Experiments with Postgres and SSL
Previous Message Jelte Fennema-Nio 2023-12-30 17:57:03 Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs