From d1aedcf9656809a74723166620d83111f8a4db2a Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Tue, 27 Oct 2026 20:13:23 +0000 Subject: [PATCH v5 1/1] pgstat: Add kind filter to pgstat_drop_matching_entries pgstat_drop_matching_entries() scans all variable-numbered stats hash tables. Some callers already know which stats kind they want to scan, and after 44539786b82 such a kind may have its own dshash. Change pgstat_drop_matching_entries() to take a stats kind argument. Passing PGSTAT_KIND_ALL keeps the existing behavior of scanning all variable-numbered stats hash tables, while passing a specific kind scans only the hash table for that kind. For kinds using the shared variable stats hash, filter entries by kind before invoking the match callback or dropping entries when no callback is provided. This breaks ABI compatibility, so extensions using pgstat_drop_matching_entries() will need to update the call accordingly. An immediate motivation is pg_stat_statements, where follow-up work can use this to drop matching entries from its stats kind. Other stats kinds that know their kind can use it as well. Add test_custom_stats coverage for kind-filtered drops, including the NULL-callback path. Also restore coverage for all-kinds scans with a non-NULL callback, which was lost when d52c24b0f80 removed the old injection_points stats test and 31280d96a64 added test_custom_stats. --- src/backend/utils/activity/pgstat_shmem.c | 37 +++++++- src/include/utils/pgstat_internal.h | 3 +- src/include/utils/pgstat_kind.h | 3 + .../test_custom_stats/t/001_custom_stats.pl | 94 ++++++++++++++++++- .../test_custom_var_stats--1.0.sql | 11 +++ .../test_custom_stats/test_custom_var_stats.c | 59 ++++++++++++ 6 files changed, 199 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 8500927a68b..5193cdc7272 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -1229,25 +1229,52 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, /* * Scan through the stats hashtables, dropping statistics if approved by the - * optional do_drop() function. + * optional do_drop() function. If do_drop is NULL, drop all matching entries. + * If kind is PGSTAT_KIND_ALL, match all kinds; otherwise only entries of + * the given kind match. */ void -pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), +pgstat_drop_matching_entries(PgStat_Kind kind, + bool (*do_drop) (PgStatShared_HashEntry *, Datum), Datum match_data) { + bool filter_by_kind = (kind != PGSTAT_KIND_ALL); + dshash_table **hashes; + int num_hashes; dshash_seq_status hstat; PgStatShared_HashEntry *ps; uint64 not_freed_count = 0; - for (int h = 0; h < pgStatLocal.num_var_hashes; h++) + if (filter_by_kind) + { + Assert(kind >= PGSTAT_KIND_MIN && kind <= PGSTAT_KIND_MAX); + Assert(pgStatLocal.kind_hash[kind] != NULL); + + hashes = &pgStatLocal.kind_hash[kind]; + num_hashes = 1; + } + else + { + hashes = pgStatLocal.var_hashes; + num_hashes = pgStatLocal.num_var_hashes; + } + + for (int h = 0; h < num_hashes; h++) { /* entries are removed, take an exclusive lock */ - dshash_seq_init(&hstat, pgStatLocal.var_hashes[h], true); + dshash_seq_init(&hstat, hashes[h], true); while ((ps = dshash_seq_next(&hstat)) != NULL) { if (ps->dropped) continue; + /* + * A kind-specific scan may still use the shared hash, which can + * contain entries for other kinds. + */ + if (filter_by_kind && ps->key.kind != kind) + continue; + if (do_drop != NULL && !do_drop(ps, match_data)) continue; @@ -1278,7 +1305,7 @@ pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), void pgstat_drop_all_entries(void) { - pgstat_drop_matching_entries(NULL, 0); + pgstat_drop_matching_entries(PGSTAT_KIND_ALL, NULL, 0); } static void diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h index 21403516e28..33fc58feb99 100644 --- a/src/include/utils/pgstat_internal.h +++ b/src/include/utils/pgstat_internal.h @@ -843,7 +843,8 @@ extern void pgstat_unlock_entry(PgStat_EntryRef *entry_ref); extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid, bool missing_ok); extern void pgstat_drop_all_entries(void); -extern void pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_HashEntry *, Datum), +extern void pgstat_drop_matching_entries(PgStat_Kind kind, + bool (*do_drop) (PgStatShared_HashEntry *, Datum), Datum match_data); extern PgStat_EntryRef *pgstat_get_entry_ref_locked(PgStat_Kind kind, Oid dboid, uint64 objid, bool nowait); diff --git a/src/include/utils/pgstat_kind.h b/src/include/utils/pgstat_kind.h index 45ca599d0dd..0cf20fb7e84 100644 --- a/src/include/utils/pgstat_kind.h +++ b/src/include/utils/pgstat_kind.h @@ -20,6 +20,9 @@ #define PGSTAT_KIND_MIN 1 /* Minimum ID allowed */ #define PGSTAT_KIND_MAX 32 /* Maximum ID allowed */ +/* Value that represents all stats kinds */ +#define PGSTAT_KIND_ALL (PGSTAT_KIND_MAX + 1) + /* use 0 for INVALID, to catch zero-initialized data */ #define PGSTAT_KIND_INVALID 0 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 ccda129c4ff..b9c07664784 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 @@ -114,17 +114,42 @@ foreach my $stats_kind (@var_stats_kinds) "entry4|3|Test entry 4", "variable-sized stats of entry4 for $kind_name"); - # Test drop of variable-sized stats. + # Test drop of variable-sized stats via pgstat_drop_entry(). $node->safe_psql('postgres', q(select * from test_custom_stats_var_drop('entry3'))); $result = $node->safe_psql('postgres', q(select * from test_custom_stats_var_report('entry3'))); is($result, "", "entry3 not found after drop for $kind_name"); + + # In the shared hash case, keep a relation stats entry of a different kind + # around to exercise filtering by kind before the callback is called. + if ($own_hash eq 'f') + { + $node->safe_psql('postgres', + q(CREATE TABLE test_custom_stats_shared_hash_rel(a int); + INSERT INTO test_custom_stats_shared_hash_rel VALUES (1); + SELECT pg_stat_force_next_flush())); + $result = $node->safe_psql('postgres', + q(SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = 'test_custom_stats_shared_hash_rel')); + is($result, "1", "relation stats entry exists in shared hash for $kind_name"); + } + + # Test drop of variable-sized stats via pgstat_drop_matching_entries(). $node->safe_psql('postgres', - q(select * from test_custom_stats_var_drop('entry4'))); + q(select * from test_custom_stats_var_drop_matching('entry4'))); $result = $node->safe_psql('postgres', q(select * from test_custom_stats_var_report('entry4'))); is($result, "", "entry4 not found after drop for $kind_name"); + if ($own_hash eq 'f') + { + # The relation stats entry should survive the custom stats drop. + $result = $node->safe_psql('postgres', + q(SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = 'test_custom_stats_shared_hash_rel')); + is($result, "1", + "relation stats entry remains after custom stats drop for $kind_name"); + } # Test persistence across clean restart. $node->stop(); @@ -172,6 +197,71 @@ foreach my $stats_kind (@var_stats_kinds) is($result, "", "variable-sized stats of entry2 lost after crash recovery for $kind_name" ); + + # Test drop of custom variable-numbered stats via + # pgstat_drop_matching_entries() with an all-kinds scan and a callback. + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry5', 'Test entry 5'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry6', 'Test entry 6'))); + my $matching_all_rel = "test_custom_stats_matching_all_rel_$own_hash"; + $node->safe_psql('postgres', + "CREATE TABLE $matching_all_rel(a int); + INSERT INTO $matching_all_rel VALUES (1); + SELECT pg_stat_force_next_flush()"); + $result = $node->safe_psql('postgres', + "SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = '$matching_all_rel'"); + is($result, "1", + "relation stats entry exists before all-kinds matching drop for $kind_name"); + $node->safe_psql('postgres', + q(select test_custom_stats_var_drop_matching('entry5', true))); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry5'))); + is($result, "", + "entry5 not found after all-kinds matching drop for $kind_name"); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry6'))); + is($result, "entry6|0|Test entry 6", + "entry6 remains after all-kinds matching drop for $kind_name"); + $result = $node->safe_psql('postgres', + "SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = '$matching_all_rel'"); + is($result, "1", + "relation stats entry remains after all-kinds matching drop for $kind_name"); + + # Test drop of all custom variable-numbered stats entries via + # pgstat_drop_matching_entries() with a NULL callback. + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry5', 'Test entry 5'))); + $node->safe_psql('postgres', + q(select test_custom_stats_var_create('entry6', 'Test entry 6'))); + if ($own_hash eq 'f') + { + $node->safe_psql('postgres', + q(INSERT INTO test_custom_stats_shared_hash_rel VALUES (2); + SELECT pg_stat_force_next_flush())); + $result = $node->safe_psql('postgres', + q(SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = 'test_custom_stats_shared_hash_rel')); + is($result, "1", + "relation stats entry exists before kind drop all for $kind_name"); + } + $node->safe_psql('postgres', q(select test_custom_stats_var_drop_all())); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry5'))); + is($result, "", "entry5 not found after drop all for $kind_name"); + $result = $node->safe_psql('postgres', + q(select * from test_custom_stats_var_report('entry6'))); + is($result, "", "entry6 not found after drop all for $kind_name"); + if ($own_hash eq 'f') + { + $result = $node->safe_psql('postgres', + q(SELECT n_tup_ins FROM pg_stat_user_tables + WHERE relname = 'test_custom_stats_shared_hash_rel')); + is($result, "1", + "relation stats entry remains after kind drop all 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 2b92316679f..eb484c01280 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,17 @@ RETURNS void AS 'MODULE_PATHNAME', 'test_custom_stats_var_drop' LANGUAGE C STRICT PARALLEL UNSAFE; +CREATE FUNCTION test_custom_stats_var_drop_matching(IN name TEXT, + IN all_kinds BOOLEAN DEFAULT false) +RETURNS void +AS 'MODULE_PATHNAME', 'test_custom_stats_var_drop_matching' +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 ba6486fff6a..0bd844e3fdd 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 @@ -635,6 +635,28 @@ test_custom_stats_var_update(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +static bool +test_custom_stats_var_match_entry(PgStatShared_HashEntry *p, Datum match_data) +{ + /* For kind-specific scans, non-custom entries should not reach here. */ + if (p->key.kind != PGSTAT_KIND_TEST_CUSTOM_VAR_STATS) + elog(ERROR, + "unexpected stats kind for custom stats match callback: %u", + p->key.kind); + + return p->key.objid == DatumGetUInt64(match_data); +} + +static bool +test_custom_stats_var_match_entry_all_kinds(PgStatShared_HashEntry *p, + Datum match_data) +{ + if (p->key.kind != PGSTAT_KIND_TEST_CUSTOM_VAR_STATS) + return false; + + return p->key.objid == DatumGetUInt64(match_data); +} + /* * test_custom_stats_var_drop * Remove custom statistic entry @@ -655,6 +677,43 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * test_custom_stats_var_drop_matching + * Remove custom statistic entry using a matching callback + * + * Drops the named statistic from shared memory. + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_drop_matching); +Datum +test_custom_stats_var_drop_matching(PG_FUNCTION_ARGS) +{ + char *stat_name = text_to_cstring(PG_GETARG_TEXT_PP(0)); + bool all_kinds = PG_NARGS() > 1 && PG_GETARG_BOOL(1); + PgStat_Kind kind = all_kinds ? PGSTAT_KIND_ALL : + PGSTAT_KIND_TEST_CUSTOM_VAR_STATS; + bool (*match_entry) (PgStatShared_HashEntry *, Datum) = + all_kinds ? test_custom_stats_var_match_entry_all_kinds : + test_custom_stats_var_match_entry; + uint64 stat_idx = PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name); + + pgstat_drop_matching_entries(kind, match_entry, UInt64GetDatum(stat_idx)); + + PG_RETURN_VOID(); +} + +/* + * test_custom_stats_var_drop_all + * Remove all custom statistic entries of this kind + */ +PG_FUNCTION_INFO_V1(test_custom_stats_var_drop_all); +Datum +test_custom_stats_var_drop_all(PG_FUNCTION_ARGS) +{ + pgstat_drop_matching_entries(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, NULL, 0); + + PG_RETURN_VOID(); +} + /* * test_custom_stats_var_reset * Reset all custom statistic entries -- 2.50.1