From: Andrei Lepikhov <lepihov@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
To: Sami Imseih <samimseih@gmail.com>
Cc: Greg Sabino Mullane <htamfids@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Nikolay Samokhvalov <nik@postgres.ai>
Cc: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Subject: Re: track generic and custom plans in pg_stat_statements
Date: Tue, 22 Jul 2025 15:44:11 +0200
Message-ID: <a0432b5d-84e1-4678-9963-bf5ecb749dd0@gmail.com> (raw)
In-Reply-To: <aH7IPp_Hku02xHP2@paquier.xyz>
References: <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>
<56d31e81-79d8-40bd-b957-0d6dc7c46589@gmail.com>
<CAA5RZ0tdc4HeHdFRvVPWBdXP1JwCSjPPgiQyFk8fwhv=4fKmjw@mail.gmail.com>
<2f2bde53-972d-402c-9da3-c5467cca00bf@gmail.com>
<CAA5RZ0tfr50T1xYET=1Ei=NkbmS3e7MFhMJTrnoLi2oO7n2JsQ@mail.gmail.com>
<064b9efd-92c9-4670-95f4-d911d4c7ddeb@gmail.com>
<CAA5RZ0ss-jmqeze0G8anztzLRJuMEgz9y3gTd-rgTc2Ejtwzvg@mail.gmail.com>
<aH7IPp_Hku02xHP2@paquier.xyz>
On 22/7/2025 01:07, Michael Paquier wrote:
> On Mon, Jul 21, 2025 at 04:47:31PM -0500, Sami Imseih wrote:
>> Last week I published a v11 that adds a field to QueryDesc, but as I thought
>> about this more, how about we just add 2 bool fields in QueryDesc->estate
>> ( es_cached_plan and es_is_generic_plan ), a field in CachedPlan (
>> is_generic_plan )
>> and after ExecutorStart, and if we have a cplan, we set the
>> appropriate plan cache
>> mode value. I think it's better to confine these new fields in Estate
>> rather than
>> at a higher level like QueryDesc. See v12.
>
> Yes, I think that this is a much better idea to isolate the whole
> concept and let pgss grab these values. We have lived with such
> additions for monitoring in EState a few times already, see for
> example de3a2ea3b264 and 1d477a907e63 that are tainted with my
> fingerprints.
I would like to oppose to the current version a little.
Commits de3a2ea3b264 and 1d477a907e63 introduced elements that are more
closely related to the execution phase. While the parameter
es_parallel_workers_to_launch could be considered a planning parameter,
es_parallel_workers_launched and es_total_processed should not.
Furthermore, any new code that utilises ExecutorStart/Run/End must
ensure that the fields es_cached_plan and es_is_generic_plan are set
correctly.
IMO, the concept of a generic or custom plan is a fundamental property
of the plan and should be stored within it - essentially in the
PlannedStmt structure.
Additionally, it is unclear why we incur significant costs to alter the
interface of ExplainOnePlan solely to track these parameters.
It may be more efficient to set the is_generic_plan option at the top
plan node (PlannedStmt) and reference it wherever necessary. To identify
a cached plan, we may consider pushing the CachedPlan/CachedPlanSource
pointer down throughout pg_plan_query and maintaining a reference to the
plan (or simply setting a boolean flag) at the same location — inside
the PlannedStmt.
Such knowledge may provide an extra profit for planning - a generic
plan, stored in the plan cache, may be reused later, and it may be
profitable for an analytics-related extension to spend extra cycles and
apply optimisation tricks more boldly.
--
regards, Andrei Lepikhov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: lepihov@gmail.com, michael@paquier.xyz, samimseih@gmail.com, htamfids@gmail.com, nik@postgres.ai, ilya.evdokimov@tantorlabs.com
Subject: Re: track generic and custom plans in pg_stat_statements
In-Reply-To: <a0432b5d-84e1-4678-9963-bf5ecb749dd0@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox