From ceb13948d0d197fef321967f5b1630943ecb4b60 Mon Sep 17 00:00:00 2001 From: Sami Imseih Date: Tue, 27 Oct 2026 20:13:23 +0000 Subject: [PATCH v3 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. 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, including the NULL-callback path. --- src/backend/utils/activity/pgstat_shmem.c | 31 ++++++++-- src/include/utils/pgstat_internal.h | 3 +- .../test_custom_stats/t/001_custom_stats.pl | 62 ++++++++++++++++++- .../test_custom_var_stats--1.0.sql | 10 +++ .../test_custom_stats/test_custom_var_stats.c | 48 ++++++++++++++ 5 files changed, 146 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index 8500927a68b..6a436a06ae7 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -1229,25 +1229,46 @@ 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); + int num_hashes = filter_by_kind ? 1 : pgStatLocal.num_var_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); + } + + for (int h = 0; h < num_hashes; h++) + { + dshash_table *hash = filter_by_kind ? + pgStatLocal.kind_hash[kind] : pgStatLocal.var_hashes[h]; + /* entries are removed, take an exclusive lock */ - dshash_seq_init(&hstat, pgStatLocal.var_hashes[h], true); + dshash_seq_init(&hstat, hash, 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 +1299,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 ccda129c4ff..3d57895446e 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,39 @@ 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 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..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,16 @@ 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' +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..dfc7d7e0808 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,21 @@ 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) +{ + /* + * pgstat_drop_matching_entries() should filter by kind before invoking + * the callback. + */ + 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 @@ -655,6 +670,39 @@ 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)); + 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_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