Re: pg_stat_statements: more test coverage

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 requirements because of some internal company projects.
I'm pretty sure to have proposed in the past at least one patch that
would make use of that, but it got rejected. It is not the only code
area that has a similar pattern. So why introducing that now?

> Subject: [PATCH v1 2/5] pg_stat_statements: Add coverage for
> pg_stat_statements_1_8()
>
> [...]
>
> Subject: [PATCH v1 3/5] pg_stat_statements: Add coverage for
> pg_stat_statements_reset_1_7

Yep, why not.

> +SELECT format('create table t%s (a int)', lpad(i::text, 3, '0')) FROM generate_series(1, 101) AS x(i) \gexec
> +create table t001 (a int)
> [...]
> +create table t101 (a int)

That's a lot of bloat. This relies on pg_stat_statements.max's
default to be at 100. Based on the regression tests, the maximum
number of rows we have reported from the view pg_stat_statements is
39 in utility.c. I think that you should just:
- 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:
CREATE OR REPLACE FUNCTION create_tables(num_tables int)
RETURNS VOID AS
$func$
BEGIN
FOR i IN 1..num_tables LOOP
EXECUTE format('
CREATE TABLE IF NOT EXISTS %I (id int)', 't_' || i);
END LOOP;
END
$func$ LANGUAGE plpgsql;
- Switch the minimum to be around 40~50 in the local
pg_stat_statements.conf used for the regression tests.

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

You could fetch the max value in a \get and reuse it here, as well.

> +is( $node->safe_psql(
> + 'postgres',
> + "SELECT count(*) FROM pg_stat_statements WHERE query LIKE '%t1%'"),
> + '2',
> + 'pg_stat_statements data kept across restart');

Checking that the contents match would be a bit more verbose than just
a count. One trick I've used in the patch is in
027_stream_regress.pl, where there is a query grouping the stats
depending on the beginning of the queries. Not exact, a bit more
verbose.
--
Michael