From: | Lukas Fittl <lukas(at)fittl(dot)com> |
---|---|
To: | Sami Imseih <samimseih(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, 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-02-05 02:09:50 |
Message-ID: | CAP53PkxocbNr+eRag3FEJp3-7S1U80FspOg8UQjO902TWMG=6A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Looks like some emails were sent before I could send my draft email, but
hopefully this should make the follow up work easier :)
Attached a v4 patch set with a few minor changes to plan ID jumbling:
* Range table jumbling is now done in a separate JumbleRangeTable function
after setrefs.c walked the tree - this way we avoid having custom logic for
RT Indexes in the node jumbling, and keeping a reference to PlannerGlobal
in the jumble struct
* Moved the JumbleNode call to the bottom of the set_plan_references
function for clarity - previously it was before descending into inner/outer
plan, but after some other recursive calls to set_plan_references, which
didn't really make sense
* Fixed a bug with JUMBLE_ARRAY incorrectly taking the reference of the
array (which caused planid to change incorrectly between runs)
* Added JUMBLE_BITMAPSET
Further, I've significantly reduced the number of fields ignored for plan
jumbling:
Basically the approach taken in this version is that only things that would
negatively affect the planid (i.e. make it unique when it shouldn't be) are
ignored, vs ignoring duplicate fields and fields that are only used by the
executor (which is what v1-v3 did). I'm not 100% sure that's the right
approach (but it does keep the diff a good amount smaller), I think the
tradeoff here is basically jumbling performance vs maintenance overhead
when fields are added/changed.
This does not yet move field-specific comments to their own line in nodes
where we're adding node attributes, I'll leave that for Sami to work on.
On Tue, Feb 4, 2025 at 3:15 PM Sami Imseih <samimseih(at)gmail(dot)com> wrote:
> >> Separately I've been thinking how we could best have a
> discussion/review on
> >> whether the jumbling of specific plan struct fields is correct. I was
> >> thinking maybe a quick wiki page could be helpful, noting why to
> jumble/not
> >> jumble certain fields?
>
> > Makes sense. This is a complicated topic.
>
> +1 for the Wiki page
>
Started here: https://wiki.postgresql.org/wiki/Plan_ID_Jumbling
Thanks,
Lukas
--
Lukas Fittl
Attachment | Content-Type | Size |
---|---|---|
v4-0002-Optionally-record-a-plan_id-in-PlannedStmt-to-ide.patch | application/octet-stream | 54.6 KB |
v4-0003-Add-pg_stat_plans-contrib-extension.patch | application/octet-stream | 71.0 KB |
v4-0001-Allow-using-jumbling-logic-outside-of-query-jumbl.patch | application/octet-stream | 4.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Lukas Fittl | 2025-02-05 02:16:07 | Re: [PATCH] Optionally record Plan IDs to track plan changes for a query |
Previous Message | Jeff Davis | 2025-02-05 01:40:41 | Re: new commitfest transition guidance |