On Fri, Oct 30, 2026 at 09:41:12AM -0500, Sami Imseih wrote: > Hmm, yeah. v5 adds PGSTAT_KIND_ALL and uses that for the all-kinds > case.
PGSTAT_KIND_ALL holds the same meaning as PGSTAT_KIND_INVALID on an
interface-basis. I'd just rely on PGSTAT_KIND_INVALID, drop
PGSTAT_KIND_ALL; that seems simpler overall.
> Adding an all-kinds scan with a non-NULL callback did cross my mind,
> but I did not want to overdo the tests. I also did not realize that this
> used to be covered by the injection_points test, so thanks for the
> research.
>
> v5 now has these pgstat_drop_matching_entries() cases:
>
> 1. Specific kind + non-NULL callback
> 2. All kinds + non-NULL callback (similar to the injection points test
> functionally)
> 3. Specific kind + NULL callback
The NULL callback case is already covered by the in-core code, for
example where we drop all the entries during a reset. I don't see a
specific need for more coverage with this pattern.
-static bool
-match_inj_entries(PgStatShared_HashEntry *entry, Datum match_data)
-{
- return entry->key.kind == PGSTAT_KIND_INJECTION;
-}
[...]
-injection_points_stats_drop(PG_FUNCTION_ARGS)
-{
- pgstat_drop_matching_entries(match_inj_entries, 0);
-
- PG_RETURN_VOID();
Hmmm... Indeed... We used to have a pgstat_drop_matching_entries()
call that we have dropped when moving these tests from
injection_points to test_custom_stats. That was not intentional.
I think that your test_custom_stats_var_drop_all() makes sense, with
a callback that matches with the variable-sized kind ID. This should
be backpatched down to v19, perhaps, but I'm mild with playing with
this stable branch as we are close to release. And this is just for a
small gap in the tests.
Anyway, what do you think about the attached to address this hole in
the tests? I'll continue my review of the main patch once this issue
is fixed.
--
Michael
From 2ad407d2a56240542049211cf9cfe6e4c3d6eb03 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Mon, 2 Nov 2026 09:28:17 +0900 Subject: [PATCH] test_custom_stats: Add test for dropped stats --- .../test_custom_stats/t/001_custom_stats.pl | 15 ++++++++++++ .../test_custom_var_stats--1.0.sql | 5 ++++ .../test_custom_stats/test_custom_var_stats.c | 23 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/src/test/modules/test_custom_stats/t/001_custom_stats.pl b/src/test/modules/test_custom_stats/t/001_custom_stats.pl index ccda129c4ffc..d49c02badbac 100644 --- a/src/test/modules/test_custom_stats/t/001_custom_stats.pl +++ b/src/test/modules/test_custom_stats/t/001_custom_stats.pl @@ -172,6 +172,21 @@ foreach my $stats_kind (@var_stats_kinds) is($result, "", "variable-sized stats of entry2 lost after crash recovery for $kind_name" ); + + # Test drop of all variable-sized stats. + $node->safe_psql( + 'postgres', + q(create table if not exists test_drop_all (a int); + insert into test_drop_all values (1); + select test_custom_stats_var_create('entry5', 'Test entry 5'))); + $node->safe_psql('postgres', q(select test_custom_stats_var_drop_all())); + $result = $node->safe_psql('postgres', + qq(select entry_count from pg_stat_kind_info where name = '$kind_name')); + is($result, "0", "no entries after drop for $kind_name"); + $result = $node->safe_psql('postgres', + q(select n_tup_ins from pg_stat_user_tables + where relname = 'test_drop_all')); + is($result, "1", "relation stats remain after drop for $kind_name"); } # fixed-sized stats are updated 3 times, so the report below should match. diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql b/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql index 2b92316679fb..d758968b12c8 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql +++ b/src/test/modules/test_custom_stats/test_custom_var_stats--1.0.sql @@ -18,6 +18,11 @@ RETURNS void AS 'MODULE_PATHNAME', 'test_custom_stats_var_drop' LANGUAGE C STRICT PARALLEL UNSAFE; +CREATE FUNCTION test_custom_stats_var_drop_all() +RETURNS void +AS 'MODULE_PATHNAME', 'test_custom_stats_var_drop_all' +LANGUAGE C STRICT PARALLEL UNSAFE; + CREATE FUNCTION test_custom_stats_var_reset() RETURNS void AS 'MODULE_PATHNAME', 'test_custom_stats_var_reset' diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/src/test/modules/test_custom_stats/test_custom_var_stats.c index ba6486fff6a4..8be4abe2f827 100644 --- a/src/test/modules/test_custom_stats/test_custom_var_stats.c +++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c @@ -655,6 +655,29 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * Callback for pgstat_drop_matching_entries(). + */ +static bool +test_custom_stats_var_match_kind(PgStatShared_HashEntry *entry, + Datum match_data) +{ + return entry->key.kind == PGSTAT_KIND_TEST_CUSTOM_VAR_STATS; +} + +/* + * test_custom_stats_var_drop_all + * Remove all custom statistic entries + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_drop_all); +Datum +test_custom_stats_var_drop_all(PG_FUNCTION_ARGS) +{ + pgstat_drop_matching_entries(test_custom_stats_var_match_kind, 0); + + PG_RETURN_VOID(); +} + /* * test_custom_stats_var_reset * Reset all custom statistic entries -- 2.56.0
signature.asc
Description: PGP signature
