On 2022-Dec-09, Amit Langote wrote:
> On Fri, Dec 9, 2022 at 6:52 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Remind me again why is part_prune_results_list not part of struct
> > CachedPlan then? I tried to understand that based on comments upthread,
> > but I was unable to find anything.
>
> It used to be part of CachedPlan for a brief period of time (in patch
> v12 I posted in [1]), but David, in his reply to [1], said he wasn't
> so sure that it belonged there.
I'm not sure I necessarily agree with that. I'll have a look at v12 to
try and understand what was David so unhappy about.
> > (My first reaction to your above comment was "well, rename GetCachedPlan
> > then, maybe to GetRunnablePlan", but then I'm wondering if CachedPlan is
> > in any way a structure that must be "immutable" in the way parser output
> > is. Looking at the comment at the top of plancache.c it appears to me
> > that it isn't, but maybe I'm missing something.)
>
> CachedPlan *is* supposed to be read-only per the comment above
> CachedPlanSource definition:
>
> * ...If we are using a generic
> * cached plan then it is meant to be re-used across multiple executions, so
> * callers must always treat CachedPlans as read-only.
I read that as implying that the part_prune_results_list must remain
intact as long as no invalidations occur. Does part_prune_result_list
really change as a result of something other than a sinval event?
Keep in mind that if a sinval message that touches one of the relations
in the plan arrives, then we'll discard it and generate it afresh. I
don't see that the part_prune_results_list would change otherwise, but
maybe I misunderstand?
> FYI, there was even an idea of putting a PartitionPruneResults for a
> given PlannedStmt into the PlannedStmt itself [2], but PlannedStmt is
> supposed to be read-only too [3].
Hmm, I'm not familiar with PlannedStmt lifetime, but I'm definitely not
betting that Tom is wrong about this.
> Maybe we need some new overarching context when invoking plancache, if
> Portal can't already be it, whose struct can be passed to
> GetCachedPlan() to put the pruning results in? Perhaps,
> GetRunnablePlan() that you floated could be a wrapper for
> GetCachedPlan(), owning that new context.
Perhaps that is a solution. I'm not sure.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Uno puede defenderse de los ataques; contra los elogios se esta indefenso"
