On Tue, Jul 22, 2025 at 02:52:49PM -0500, Sami Imseih wrote:
>> We will need a field to store an enum. let's call it CachedPlanType
>> with the types of cached plan. We need to be able to differentiate
>> when cached plans are not used, so a simple boolean is not
>> sufficient.
Guess so.
>> We can track a field for this enum in PlannedStmt and initialize
>> it to PLAN_CACHE_NOT_SET whenever we makeNode(PlannedStmt).
>> In GetCachedPlan, we iterate through plan->stmt_list to set the
>> value for the PlannedStmt.
>>
>> CachedPlanType will have to be defined in nodes.h for this to work.
>>
>> We can avoid the struct altogether and define the new PlannedStmt
>> field as an "int" and bit-pack the types.
>>
>> I prefer the CachedPlanType struct to do this, as it's cleaner and
>> self-documenting.
>
> v13 is the implementation using PlannedStmt as described above.
+ PLAN_CACHE_NOT_SET, /* Not a cached plan */
This should be set to 0 by default, but we could enforce the
definition as well with a PLAN_CACHE_NOT_SET = 0? Nit: perhaps name
it PLAN_CACHE_NONE to outline the fact that it is just not a cached
plan?
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -131,6 +131,9 @@ typedef struct PlannedStmt
/* non-null if this is utility stmt */
Node *utilityStmt;
+ /* type of cached plan, if it is one */
+ CachedPlanType cached_plan_type;
[...]
+ foreach(lc, plan->stmt_list)
+ {
+ PlannedStmt *pstmt = (PlannedStmt *) lfirst(lc);
+
+ pstmt->cached_plan_type = customplan ? PLAN_CACHE_CUSTOM : PLAN_CACHE_GENERIC;
+ }
Yes, this implementation feels much more natural, with a state
set to the result that we get when directly retrieving it from the
cached plan layer, pushing the data in the executor end hook in PGSS.
No objections to this approach; I can live with that.
A small thing that would be cleaner is to split the patch into two
parts: one for the in-core backend addition and a second for PGSS.
Code paths are different, so it's simple to do.
Andrei, what do you think about all that?
--
Michael
