Re: pg_stat_statements: more test coverage

2023-12-31 Thread Tom Lane
Peter Eisentraut writes: > It looks like the failing configurations are exactly all the big-endian > ones: s390x, sparc, powerpc. So it's possible that this is actually a > bug? But unless someone can reproduce this locally and debug it, we > should probably revert this for now. The reason

Re: pg_stat_statements: more test coverage

2023-12-31 Thread Tom Lane
Peter Eisentraut writes: > It looks like the failing configurations are exactly all the big-endian > ones: s390x, sparc, powerpc. So it's possible that this is actually a > bug? But unless someone can reproduce this locally and debug it, we > should probably revert this for now. I see it

Re: pg_stat_statements: more test coverage

2023-12-31 Thread Peter Eisentraut
On 31.12.23 10:26, Julien Rouhaud wrote: On Sun, Dec 31, 2023 at 2:28 PM Michael Paquier wrote: On Sat, Dec 30, 2023 at 08:39:47PM +0100, Peter Eisentraut wrote: Ok, I have committed these two patches. Please note that the buildfarm has turned red, as in:

Re: pg_stat_statements: more test coverage

2023-12-31 Thread Julien Rouhaud
On Sun, Dec 31, 2023 at 2:28 PM Michael Paquier wrote: > > On Sat, Dec 30, 2023 at 08:39:47PM +0100, Peter Eisentraut wrote: > > Ok, I have committed these two patches. > > Please note that the buildfarm has turned red, as in: >

Re: pg_stat_statements: more test coverage

2023-12-30 Thread Michael Paquier
On Sat, Dec 30, 2023 at 08:39:47PM +0100, Peter Eisentraut wrote: > Ok, I have committed these two patches. Please note that the buildfarm has turned red, as in: https://buildfarm.postgresql.org/cgi-bin/show_stagxe_log.pl?nm=pipit=2023-12-31%2001%3A12%3A22=misc-check pg_stat_statements's

Re: pg_stat_statements: more test coverage

2023-12-30 Thread Michael Paquier
On Sat, Dec 30, 2023 at 08:39:47PM +0100, Peter Eisentraut wrote: > On 29.12.23 06:14, Julien Rouhaud wrote: >> 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. Just to

Re: pg_stat_statements: more test coverage

2023-12-30 Thread Peter Eisentraut
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?

Re: pg_stat_statements: more test coverage

2023-12-28 Thread Julien Rouhaud
On Wed, Dec 27, 2023 at 8:53 PM Peter Eisentraut 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

Re: pg_stat_statements: more test coverage

2023-12-27 Thread Peter Eisentraut
On 27.12.23 09:08, Julien Rouhaud wrote: Hi, On Tue, Dec 26, 2023 at 10:03 PM Peter Eisentraut wrote: On 24.12.23 03:03, Michael Paquier wrote: - Use a DO block of a PL function, say with something like that to ensure an amount of N queries? Say with something like that after tweaking

Re: pg_stat_statements: more test coverage

2023-12-27 Thread Julien Rouhaud
Hi, On Tue, Dec 26, 2023 at 10:03 PM Peter Eisentraut wrote: > > On 24.12.23 03:03, Michael Paquier wrote: > > - Use a DO block of a PL function, say with something like that to > > ensure an amount of N queries? Say with something like that after > > tweaking pg_stat_statements.track: > >

Re: pg_stat_statements: more test coverage

2023-12-26 Thread Peter Eisentraut
On 24.12.23 03:03, Michael Paquier wrote: On Sat, Dec 23, 2023 at 03:18:01PM +0100, Peter Eisentraut wrote: +/* LCOV_EXCL_START */ +PG_FUNCTION_INFO_V1(pg_stat_statements); PG_FUNCTION_INFO_V1(pg_stat_statements_1_2); +/* LCOV_EXCL_STOP */ The only reason why I've seen this used at the C

Re: pg_stat_statements: more test coverage

2023-12-23 Thread Michael Paquier
On Sat, Dec 23, 2023 at 03:18:01PM +0100, Peter Eisentraut wrote: > +/* LCOV_EXCL_START */ > +PG_FUNCTION_INFO_V1(pg_stat_statements); > PG_FUNCTION_INFO_V1(pg_stat_statements_1_2); > +/* LCOV_EXCL_STOP */ The only reason why I've seen this used at the C level was to bump up the coverage

pg_stat_statements: more test coverage

2023-12-23 Thread Peter Eisentraut
pg_stat_statements has some significant gaps in test coverage, especially around the serialization of data around server restarts, so I wrote a test for that and also made some other smaller tweaks to increase the coverage a bit. These patches are all independent of each other. After that,