On 2022-Dec-09, Amit Langote wrote: > On Fri, Dec 9, 2022 at 6:52 PM Alvaro Herrera <alvhe...@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"