Message-ID: <56d31e81-79d8-40bd-b957-0d6dc7c46589@gmail.com>
Date: Thu, 17 Jul 2025 09:48:55 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: track generic and custom plans in pg_stat_statements
To: Sami Imseih <samimseih@gmail.com>
Cc: Greg Sabino Mullane <htamfids@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
 Nikolay Samokhvalov <nik@postgres.ai>,
 Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
References: 
 <CAA5RZ0uFw8Y9GCFvafhC=OA8NnMqVZyzXPfv_EePOt+iv1T-qQ@mail.gmail.com>
 <b33a4d04-6cbd-49d4-ba9f-b2b001a154d5@tantorlabs.com>
 <CAA5RZ0vqVPD0-h+MukmNBkfP11oV_2=T+pKidyeUQbBWRTMbmA@mail.gmail.com>
 <CAA5RZ0sxV3APLzmnH1Jt0ehmjzYAStNqcZeEAma_D0VkmjkX8w@mail.gmail.com>
 <1d667f28-c739-417b-b024-7f5c1004a008@tantorlabs.com>
 <CAA5RZ0sUcVngZHLG_Q_N=OD9QhC+SJf0dx7HsPX9U4SJQF20gA@mail.gmail.com>
 <CAA5RZ0t5w8mx5hb9KEFv29FFK+AScOSUuXE4raQ_pHQTFQUotw@mail.gmail.com>
 <CAM527d9jo-EPHwVwGym3fxmiebtKhhM9q8F5WGsK7S4XCsMGrA@mail.gmail.com>
 <CAA5RZ0vGBFHvGb4iZdmSADuybkHGNrxVHknTGaEhS2=2LcM56g@mail.gmail.com>
 <CAA5RZ0tpO_LwFNW3UJGLfK2_BNS+bgKKrKS10M1saOoGW42sKw@mail.gmail.com>
 <60f09467-bfe6-49bc-a47a-fe887050a23a@tantorlabs.com>
 <CAA5RZ0v4Lqc3KOzFCOJd0hjBpF10W=-E6xn0z_HizZY9uxPR0w@mail.gmail.com>
 <CAA5RZ0vK_fah+ZS00hdg9uH2J_zBgV75x6ojYrc6+OJudnbKtQ@mail.gmail.com>
 <5f775987-8a93-452d-bb3f-a1de53836406@gmail.com>
 <CAA5RZ0uxcVu9MPXQc6qJ5mtYhH=_7b7jjaDnvg6Sft8jJ1NbXw@mail.gmail.com>
Content-Language: en-US
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CAA5RZ0uxcVu9MPXQc6qJ5mtYhH=_7b7jjaDnvg6Sft8jJ1NbXw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/56d31e81-79d8-40bd-b957-0d6dc7c46589%40gmail.com>
Precedence: bulk

On 17/7/2025 00:58, Sami Imseih wrote:
>> this for better tracking. By adding a CachedPlanSource::cplan link, we
>> can establish a connection to the entire PlanCache entry instead of only
>> CachedPlan within a queryDesc and, consequently, make it accessible from
>> the executor. This would give an access to statistics on costs and the
>> number of replannings.
> 
> This maybe out of scope for this patch, but can you elaborate on what you mean
> by "CachedPlanSource::cplan link" ?
You need to introduce a 'status' field, right? - to allow someone to 
identify the plan's type, which was previously somewhat complicated. 
However, it may be implemented in a slightly different way, by adding 
CachedPlanSource::cplan (meaning 'Current Plan') and a trivial 
convention: 'cplan' references the gplan field or it refers a custom 
plan. Instead of the CachedPlan, we may provide the executor with a link 
to more stable and informative CachedPlanSource entry. That's the main idea.
As I see it, CachedPlan doesn't make sense without plancache and a 
CachedPlanSource entry. So, it is at least a valid solution.

With that link, you can access various statistics: num_custom_plans, 
num_generic_plans, total_custom_cost, and generic_cost. It would also be 
possible to clear the *_cost fields and allow Postgres to make a new 
attempt at choosing the plan type - who knows, maybe the previous 
decision is already outdated?

My point is that we can address one of the common issues with generic 
plans in a more extensible way, enabling modules to access the 
CachedPlanSource data at the time they have access to the execution 
instrumentation.

It seems impractical to me to invent one more patch: since you've 
already modified the CreateQueryDesc interface and introduced a plan 
type identification logic, why do it twice?

-- 
regards, Andrei Lepikhov