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

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Lukas Fittl <lukas@fittl.com>
Cc: Sami Imseih <samimseih@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Marko M <marko@pganalyze.com>
Date: 2025-01-27T03:53:36Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. doc: Update header file mention for CompareType

  2. Allow plugins to set a 64-bit plan identifier in PlannedStmt

  3. Fix indentation of comment in plannodes.h

  4. Reformat node comments in plannodes.h

  5. injection_points: Add routine able to drop all stats

  6. Add pgstat_drop_matching_entries() to pgstats

  7. Move pg_stat_statements query jumbling to core.