From 57672d38b7184ec7a43241d2ee1133ad23fd9283 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Mon, 2 Nov 2026 14:26:49 +0000 Subject: [PATCH v6 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 such a kind may have its own dshash. Change pgstat_drop_matching_entries() to take a stats kind argument. PGSTAT_KIND_INVALID preserves the existing all-kinds behavior, while a specific kind scans the hash mapped to that kind. When that hash is shared by multiple stats kinds, filter entries by kind before invoking the match callback or applying the NULL-callback drop-all behavior. 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. Author: Sami Imseih Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/CAN12%2BYL7SrAwzMpzfNLrVcy%3Dw5L03p7czyO73RfS06kmdqs_fw%40mail.gmail.com --- src/backend/utils/activity/pgstat_shmem.c | 37 ++++++++++++--- src/include/utils/pgstat_internal.h | 3 +- .../test_custom_stats/t/001_custom_stats.pl | 29 +++++++++++- .../test_custom_var_stats--1.0.sql | 5 +++ .../test_custom_stats/test_custom_var_stats.c | 45 ++++++++++++++++++- 5 files changed, 110 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 8500927a68b..524a0db3809 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_INVALID, 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_INVALID); + 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_INVALID, 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/test/modules/test_custom_stats/t/001_custom_stats.pl b/src/test/modules/test_custom_stats/t/001_custom_stats.pl index 4ea4aba9023..290e4175a31 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(); 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 d758968b12c..d58a310aa14 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_matching(IN name TEXT) +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' 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 8be4abe2f82..d572639e4ed 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,24 @@ test_custom_stats_var_update(PG_FUNCTION_ARGS) PG_RETURN_VOID(); } +/* + * test_custom_stats_var_match_entry + * Match custom variable stats entries by objid + * + * Callback for pgstat_drop_matching_entries(). Verifies that it filters + * entries by kind before invoking the callback. + */ +static bool +test_custom_stats_var_match_entry(PgStatShared_HashEntry *p, Datum match_data) +{ + 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); +} + /* * test_custom_stats_var_drop * Remove custom statistic entry @@ -656,6 +674,29 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS) } /* + * test_custom_stats_var_drop_matching + * Remove custom statistic entry by matching objid + * + * Drops the named statistic using pgstat_drop_matching_entries(). + */ +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)); + uint64 stat_idx = PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name); + + pgstat_drop_matching_entries(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, + test_custom_stats_var_match_entry, + UInt64GetDatum(stat_idx)); + + PG_RETURN_VOID(); +} + +/* + * test_custom_stats_var_match_kind + * Match custom variable stats entries by kind + * * Callback for pgstat_drop_matching_entries(). */ static bool @@ -673,7 +714,9 @@ 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); + pgstat_drop_matching_entries(PGSTAT_KIND_INVALID, + test_custom_stats_var_match_kind, + 0); PG_RETURN_VOID(); } -- 2.50.1