Message-ID: <5fd9a3d8-8c4e-470a-9146-164a77c75f79@gmail.com>
Date: Fri, 24 Jan 2025 16:23:08 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] Optionally record Plan IDs to track plan changes for a
 query
To: Lukas Fittl <lukas@fittl.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Marko M <marko@pganalyze.com>, Sami Imseih <samimseih@gmail.com>
References: <CAP53Pkyow59ajFMHGpmb1BK9WHDypaWtUsS_5DoYUEfsa_Hktg@mail.gmail.com>
Content-Language: en-AU
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: <CAP53Pkyow59ajFMHGpmb1BK9WHDypaWtUsS_5DoYUEfsa_Hktg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/5fd9a3d8-8c4e-470a-9146-164a77c75f79%40gmail.com>
Precedence: bulk

On 1/3/25 03:46, Lukas Fittl wrote:
> My overall perspective is that (1) is best done in-core to keep overhead 
> low, whilst (2) could be done outside of core (or merged with a future 
> pg_stat_statements) and is included here mainly for illustration purposes.
Thank you for the patch and your attention to this issue!

I am pleased with the export of the jumbling functions and their 
generalisation.

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).

Overall, similar to discussions on queryId, various extensions may want 
different logic for generating plan_id (more or less unique guarantees, 
for example). Hence, it would be beneficial to separate this logic and 
allow extensions to provide different plan_ids. IMO, What we need is a 
'List *ext' field in each of the Plan, Path, PlanStmt, and Query 
structures. Such 'ext' field may contain different stuff that extensions 
want to push without interference between them - specific plan_id as an 
example.

Additionally, we could bridge the gap between the cloud of paths and the 
plan by adding a hook at the end of the create_plan_recurse routine. 
This may facilitate the transfer of information regarding optimiser 
decisions that could be influenced by an extension into the plan.

-- 
regards, Andrei Lepikhov