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

On Mon, Jan 27, 2025 at 12:53:36PM +0900, Michael Paquier wrote:
> 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.

After thinking more about this one, I still want this toy and hearing
nothing I have applied it, with a second commit for the addition in
injection_points to avoid multiple bullet points in a single commit.

I have noticed post-commit that I have made a mistake in the credits
of a632cd354d35 and ce5c620fb625 for your family name.  Really sorry
about that!  This mistake is on me..

> What do you think?

Attached is a rebased version of the three remaining patches.  While
looking at this stuff, I have noticed an extra cleanup that would be
good to have, as a separate change: we could reformat a bit the plan
header comments so as these do not require a rewrite when adding
node_attr to them, like d575051b9af9.

Sami's patch set posted at [1] has the same problem, making the
proposals harder to parse and review, and the devil is in the details
with these pg_node_attr() properties attached to the structures.  That
would be something to do on top of the proposed patch sets.  Would any
of you be interested in that?

[1]: https://www.postgresql.org/message-id/CAA5RZ0sUPPOpkRZD=Za83op2ngcPC7dp249vcHA-X5YS7p3n8Q@mail.gmail.com
--
Michael