Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash

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