Re: [PATCH] Optionally record Plan IDs to track plan changes for a query

On Fri, Jan 24, 2025 at 01:59:00AM -0800, Lukas Fittl wrote:
> Overall, I also do wonder if it wouldn't be better to have a callback
> mechanism in the shared memory stats, so stats plugins can do extra work
> when an entry gets dropped (like freeing the DSA memory for the plan text),
> vs having to add all this extra logic to do it.

Not sure about this part yet. I have looked at 0002 to begin with
something and it is really useful on its own. Stats kinds calling
this routine don't need to worry about the internals of dropping local
references or doing a seqscan on the shared hash table. However, what
you have sent lacks in flexibility to me, and the duplication with
pgstat_drop_all_entries is annoying. This had better be merged in a
single routine.

Attached is an updated version that adds an optional "do_drop"
callback in the function that does the seqscan on the dshash, to
decide if an entry should be gone or not. This follows the same model
as the "reset" part, where stats kind can push the matching function
they want to work on the individual entries. We could add a
pgstat_drop_entries_of_kind(), but I'm not feeling that this is
strongly necessary with the basic interface in place.

The changes in the module injection_points were not good. The SQL
function was named "reset" but that's a drop operation.

What do you think?
--
Michael