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