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

On 2/5/25 09:16, Lukas Fittl wrote:
> Hi Andrei,
>
> On Fri, Jan 24, 2025 at 1:23 AM Andrei Lepikhov <lepihov(at)gmail(dot)com
> <mailto:lepihov(at)gmail(dot)com>> wrote:
>
> I may not be close to the task monitoring area, but I utilise queryId
> and other tools to differ plan nodes inside extensions. Initially, like
> queryId serves as a class identifier for queries, plan_id identifies a
> class of nodes, not a single node. In the implementation provided here,
> nodes with the same hash can represent different subtrees. For example,
> JOIN(A, JOIN(B,C)) and JOIN(JOIN(B,C),A) may have the same ID.
>
>
> Moreover, I wonder if this version of plan_id reacts to the join level
> change. It appears that only a change of the join clause alters the
> plan_id hash value, which means you would end up with a single hash for
> very different plan nodes. Is that acceptable? To address this, we
> should consider the hashes of the left and right subtrees and the
> hashes
> of each subplan (especially in the case of Append).
>
>
> I looked back at this again just to confirm we're not missing anything:
>
> I don't think any of the posted patch versions (including the just
> shared v4) have a problem with distinguishing two plans that are very
> similar but only differ in JOIN order. Since we descend into the inner/
> outer plans via the setrefs.c treewalk, the placement of JOIN nodes vs
> other nodes should cause a different plan jumble (and we include both
> the node tag for the join/scan nodes, as well as the RT index the scans
> point to in the jumble).
Maybe. I haven't dive into that stuff deeply yet. It is not difficult to
check.
The main point was that different extensions want different plan_ids.
For example, planner extensions want to guarantee the distinctness and
sort of stability of this field inside a query plan. Does the hash value
guarantee that?
We have discussed how queryId should be generated more than once. That's
why I think the plan_id generation logic should be implemented inside an
extension, not in the core.

--
regards, Andrei Lepikhov