Message-ID: <064b9efd-92c9-4670-95f4-d911d4c7ddeb@gmail.com>
Date: Mon, 21 Jul 2025 16:55:45 +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>
 <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>
 <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>
Content-Language: en-US
From: Andrei Lepikhov <lepihov@gmail.com>
In-Reply-To: 
 <CAA5RZ0tfr50T1xYET=1Ei=NkbmS3e7MFhMJTrnoLi2oO7n2JsQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/064b9efd-92c9-4670-95f4-d911d4c7ddeb%40gmail.com>
Precedence: bulk

On 18/7/2025 21:37, Sami Imseih wrote:
> Thanks for clearing up my understanding. Essentially, override the
> current cost-based
> method of determining custom vs. generic by using something like execution time,
> which is somehow tracked by the extension. That is how I understand this.
> Now, I wonder if it may be a good idea to add some hooks in GetCachedPlan
> to make this work?
Yes, it may work. But inside the GetCachedPlan, we still don't have 
access to the executed statement yet.
> 
>>> Adding CachedPlanSource to QueryDesc seems doable. However, Michael
>>> previously objected to adding CachedPlan to QueryDesc. Is there any
>>> similar hesitation about including CachedPlanSource?
>> I agree that we should investigate further to find the most optimal
>> solution. Personally, I'm open to including an internal reference to a
>> plan cache entry within the QueryDesc, as long as the plan has its roots
>> there.
> 
> For the sake of this feature, I suspect making CachedPlanSource available
> in QueryDesc will be a non-starter, but I would like to hear other opinions.
> To accomplish the goals of this patch, we definitely need the current
> execution’s
> "plan cache mode" to be accessible in ExecutorEnd somehow.
I agree with this - adding references to CachedPlan into the QueryDesc 
looks kludge.
The most boring aspect of pg_stat_statements for me is the multiple 
statements case: a single incoming query (the only case in the cache of 
a generic plan) may be rewritten as various statements with the same 
query ID. So, the execution time of the initial statement should be the 
sum of the executions of all rewritten parts.
I wonder if the pg_stat_statements processes it correctly at the moment. 
Because in this case, we'd need a hook inside the Portal execution code. 
However, I don't recall any threads on the mailing list discussing that.

If the multiple-statements case didn't exist for a cached statement, 
Michael's idea with an active portal would be much better, and it would 
make sense to add PlanCacheSource::cplan and replace Portal::CachedPlan 
with Portal::PlanCacheSource reference.

-- 
regards, Andrei Lepikhov