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
