Revert "Don't lock partitions pruned by initial pruning"

As pointed out by Tom Lane, the patch introduced fragile and invasive
design around plan invalidation handling when locking of prunable
partitions was deferred from plancache.c to the executor. In
particular, it violated assumptions about CachedPlan immutability and
altered executor APIs in ways that are difficult to justify given the
added complexity and overhead.

This also removes the firstResultRels field added to PlannedStmt in
commit 28317de72, which was intended to support deferred locking of
certain ModifyTable result relations.

Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/605328.1747710381@sss.pgh.pa.us

9 files changed, 10 insertions, 96 deletions

diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 03c5b3d73e5..3b122f79ed8 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -63,10 +63,8 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
 							  struct ExplainState *es, ParseState *pstate,
 							  ParamListInfo params);
 
-extern void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
-						   CachedPlanSource *plansource, int query_index,
-						   IntoClause *into, struct ExplainState *es,
-						   const char *queryString,
+extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
+						   struct ExplainState *es, const char *queryString,
 						   ParamListInfo params, QueryEnvironment *queryEnv,
 						   const instr_time *planduration,
 						   const BufferUsage *bufusage,
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 4180601dcd4..2ed2c4bb378 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -258,7 +258,6 @@ extern void ExecASTruncateTriggers(EState *estate,
 extern void AfterTriggerBeginXact(void);
 extern void AfterTriggerBeginQuery(void);
 extern void AfterTriggerEndQuery(EState *estate);
-extern void AfterTriggerAbortQuery(void);
 extern void AfterTriggerFireDeferred(void);
 extern void AfterTriggerEndXact(bool isCommit);
 extern void AfterTriggerBeginSubXact(void);
diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h
index ba53305ad42..86db3dc8d0d 100644
--- a/src/include/executor/execdesc.h
+++ b/src/include/executor/execdesc.h
@@ -35,7 +35,6 @@ typedef struct QueryDesc
 	/* These fields are provided by CreateQueryDesc */
 	CmdType		operation;		/* CMD_SELECT, CMD_UPDATE, etc. */
 	PlannedStmt *plannedstmt;	/* planner's output (could be utility, too) */
-	CachedPlan *cplan;			/* CachedPlan that supplies the plannedstmt */
 	const char *sourceText;		/* source text of the query */
 	Snapshot	snapshot;		/* snapshot to use for query */
 	Snapshot	crosscheck_snapshot;	/* crosscheck for RI update/delete */
@@ -58,7 +57,6 @@ typedef struct QueryDesc
 
 /* in pquery.c */
 extern QueryDesc *CreateQueryDesc(PlannedStmt *plannedstmt,
-								  CachedPlan *cplan,
 								  const char *sourceText,
 								  Snapshot snapshot,
 								  Snapshot crosscheck_snapshot,
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index ae99407db89..104b059544d 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -19,7 +19,6 @@
 #include "nodes/lockoptions.h"
 #include "nodes/parsenodes.h"
 #include "utils/memutils.h"
-#include "utils/plancache.h"
 
 
 /*
@@ -73,7 +72,7 @@
 
 
 /* Hook for plugins to get control in ExecutorStart() */
-typedef bool (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
+typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
 extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
 
 /* Hook for plugins to get control in ExecutorRun() */
@@ -229,11 +228,8 @@ ExecGetJunkAttribute(TupleTableSlot *slot, AttrNumber attno, bool *isNull)
 /*
  * prototypes from functions in execMain.c
  */
-extern bool ExecutorStart(QueryDesc *queryDesc, int eflags);
-extern void ExecutorStartCachedPlan(QueryDesc *queryDesc, int eflags,
-									CachedPlanSource *plansource,
-									int query_index);
-extern bool standard_ExecutorStart(QueryDesc *queryDesc, int eflags);
+extern void ExecutorStart(QueryDesc *queryDesc, int eflags);
+extern void standard_ExecutorStart(QueryDesc *queryDesc, int eflags);
 extern void ExecutorRun(QueryDesc *queryDesc,
 						ScanDirection direction, uint64 count);
 extern void standard_ExecutorRun(QueryDesc *queryDesc,
@@ -300,30 +296,6 @@ extern void ExecEndNode(PlanState *node);
 extern void ExecShutdownNode(PlanState *node);
 extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node);
 
-/*
- * Is the CachedPlan in es_cachedplan still valid?
- *
- * Called from InitPlan() because invalidation messages that affect the plan
- * might be received after locks have been taken on runtime-prunable relations.
- * The caller should take appropriate action if the plan has become invalid.
- */
-static inline bool
-ExecPlanStillValid(EState *estate)
-{
-	return estate->es_cachedplan == NULL ? true :
-		CachedPlanValid(estate->es_cachedplan);
-}
-
-/*
- * Locks are needed only if running a cached plan that might contain unlocked
- * relations, such as a reused generic plan.
- */
-static inline bool
-ExecShouldLockRelations(EState *estate)
-{
-	return estate->es_cachedplan == NULL ? false :
-		CachedPlanRequiresLocking(estate->es_cachedplan);
-}
 
 /* ----------------------------------------------------------------
  *		ExecProcNode
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 5b6cadb5a6c..2492282213f 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -42,7 +42,6 @@
 #include "storage/condition_variable.h"
 #include "utils/hsearch.h"
 #include "utils/queryenvironment.h"
-#include "utils/plancache.h"
 #include "utils/reltrigger.h"
 #include "utils/sharedtuplestore.h"
 #include "utils/snapshot.h"
@@ -664,7 +663,6 @@ typedef struct EState
 										 * ExecRowMarks, or NULL if none */
 	List	   *es_rteperminfos;	/* List of RTEPermissionInfo */
 	PlannedStmt *es_plannedstmt;	/* link to top of plan tree */
-	CachedPlan *es_cachedplan;	/* CachedPlan providing the plan tree */
 	List	   *es_part_prune_infos;	/* List of PartitionPruneInfo */
 	List	   *es_part_prune_states;	/* List of PartitionPruneState */
 	List	   *es_part_prune_results;	/* List of Bitmapset */
@@ -717,7 +715,6 @@ typedef struct EState
 	int			es_top_eflags;	/* eflags passed to ExecutorStart */
 	int			es_instrument;	/* OR of InstrumentOption flags */
 	bool		es_finished;	/* true when ExecutorFinish is done */
-	bool		es_aborted;		/* true when execution was aborted */
 
 	List	   *es_exprcontexts;	/* List of ExprContexts within EState */
 
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 1dd2d1560cb..6567759595d 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -138,9 +138,6 @@ typedef struct PlannerGlobal
 	/* "flat" list of integer RT indexes */
 	List	   *resultRelations;
 
-	/* "flat" list of integer RT indexes (one per ModifyTable node) */
-	List	   *firstResultRels;
-
 	/* "flat" list of AppendRelInfos */
 	List	   *appendRelations;
 
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 658d76225e4..f0d514e6e15 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -105,13 +105,6 @@ typedef struct PlannedStmt
 	/* integer list of RT indexes, or NIL */
 	List	   *resultRelations;
 
-	/*
-	 * rtable indexes of first target relation in each ModifyTable node in the
-	 * plan for INSERT/UPDATE/DELETE/MERGE
-	 */
-	/* integer list of RT indexes, or NIL */
-	List	   *firstResultRels;
-
 	/* list of AppendRelInfo nodes */
 	List	   *appendRelations;
 
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 07ec5318db7..1baa6d50bfd 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -18,8 +18,6 @@
 #include "access/tupdesc.h"
 #include "lib/ilist.h"
 #include "nodes/params.h"
-#include "nodes/parsenodes.h"
-#include "nodes/plannodes.h"
 #include "tcop/cmdtag.h"
 #include "utils/queryenvironment.h"
 #include "utils/resowner.h"
@@ -153,11 +151,10 @@ typedef struct CachedPlanSource
  * The reference count includes both the link from the parent CachedPlanSource
  * (if any), and any active plan executions, so the plan can be discarded
  * exactly when refcount goes to zero.  Both the struct itself and the
- * subsidiary data, except the PlannedStmts in stmt_list live in the context
- * denoted by the context field; the PlannedStmts live in the context denoted
- * by stmt_context.  Separate contexts makes it easy to free a no-longer-needed
- * cached plan. (However, if is_oneshot is true, the context does not belong
- * solely to the CachedPlan so no freeing is possible.)
+ * subsidiary data live in the context denoted by the context field.
+ * This makes it easy to free a no-longer-needed cached plan.  (However,
+ * if is_oneshot is true, the context does not belong solely to the CachedPlan
+ * so no freeing is possible.)
  */
 typedef struct CachedPlan
 {
@@ -165,7 +162,6 @@ typedef struct CachedPlan
 	List	   *stmt_list;		/* list of PlannedStmts */
 	bool		is_oneshot;		/* is it a "oneshot" plan? */
 	bool		is_saved;		/* is CachedPlan in a long-lived context? */
-	bool		is_reused;		/* is it a reused generic plan? */
 	bool		is_valid;		/* is the stmt_list currently valid? */
 	Oid			planRoleId;		/* Role ID the plan was created for */
 	bool		dependsOnRole;	/* is plan specific to that role? */
@@ -174,10 +170,6 @@ typedef struct CachedPlan
 	int			generation;		/* parent's generation number for this plan */
 	int			refcount;		/* count of live references to this struct */
 	MemoryContext context;		/* context containing this CachedPlan */
-	MemoryContext stmt_context; /* context containing the PlannedStmts in
-								 * stmt_list, but not the List itself which is
-								 * in the above context; NULL if is_oneshot is
-								 * true. */
 } CachedPlan;
 
 /*
@@ -249,10 +241,6 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
 								 ParamListInfo boundParams,
 								 ResourceOwner owner,
 								 QueryEnvironment *queryEnv);
-extern PlannedStmt *UpdateCachedPlan(CachedPlanSource *plansource,
-									 int query_index,
-									 QueryEnvironment *queryEnv);
-
 extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner);
 
 extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
@@ -265,30 +253,4 @@ extern bool CachedPlanIsSimplyValid(CachedPlanSource *plansource,
 extern CachedExpression *GetCachedExpression(Node *expr);
 extern void FreeCachedExpression(CachedExpression *cexpr);
 
-/*
- * CachedPlanRequiresLocking: should the executor acquire additional locks?
- *
- * If the plan is a saved generic plan, the executor must acquire locks for
- * relations that are not covered by AcquireExecutorLocks(), such as partitions
- * that are subject to initial runtime pruning.
- */
-static inline bool
-CachedPlanRequiresLocking(CachedPlan *cplan)
-{
-	return !cplan->is_oneshot && cplan->is_reused;
-}
-
-/*
- * CachedPlanValid
- *      Returns whether a cached generic plan is still valid.
- *
- * Invoked by the executor to check if the plan has not been invalidated after
- * taking locks during the initialization of the plan.
- */
-static inline bool
-CachedPlanValid(CachedPlan *cplan)
-{
-	return cplan->is_valid;
-}
-
 #endif							/* PLANCACHE_H */
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index ddee031f551..0b62143af8b 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -138,7 +138,6 @@ typedef struct PortalData
 	QueryCompletion qc;			/* command completion data for executed query */
 	List	   *stmts;			/* list of PlannedStmts */
 	CachedPlan *cplan;			/* CachedPlan, if stmts are from one */
-	CachedPlanSource *plansource;	/* CachedPlanSource, for cplan */
 
 	ParamListInfo portalParams; /* params to pass to query */
 	QueryEnvironment *queryEnv; /* environment for query */
@@ -241,8 +240,7 @@ extern void PortalDefineQuery(Portal portal,
 							  const char *sourceText,
 							  CommandTag commandTag,
 							  List *stmts,
-							  CachedPlan *cplan,
-							  CachedPlanSource *plansource);
+							  CachedPlan *cplan);
 extern PlannedStmt *PortalGetPrimaryStmt(Portal portal);
 extern void PortalCreateHoldStore(Portal portal);
 extern void PortalHashTableDeleteAll(void);