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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Lukas Fittl <lukas(at)fittl(dot)com>
Cc: Sami Imseih <samimseih(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Marko M <marko(at)pganalyze(dot)com>
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a query
Date: 2025-01-31 04:37:03
Message-ID: Z5xTb5iBHVGns35R@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v3-0001-Allow-using-jumbling-logic-outside-of-query-jumbl.patch text/x-diff 4.9 KB
v3-0002-Optionally-record-a-plan_id-in-PlannedStmt-to-ide.patch text/x-diff 70.7 KB
v3-0003-Add-pg_stat_plans-contrib-extension.patch text/x-diff 71.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sami Imseih 2025-01-31 04:48:18 Re: Proposal to Enable/Disable Index using ALTER INDEX (with patch)
Previous Message Ashutosh Bapat 2025-01-31 04:14:29 Re: Error in StrategySyncStart() prologue