Revert "Don't lock partitions pruned by initial pruning"
authorAmit Langote
Thu, 22 May 2025 05:17:24 +0000 (14:17 +0900)
committerAmit Langote
Thu, 22 May 2025 08:02:35 +0000 (17:02 +0900)
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
Discussion: https://postgr.es/m/605328.1747710381@sss.pgh.pa.us

33 files changed:
contrib/auto_explain/auto_explain.c
contrib/pg_stat_statements/pg_stat_statements.c
doc/src/sgml/release-18.sgml
src/backend/commands/copyto.c
src/backend/commands/createas.c
src/backend/commands/explain.c
src/backend/commands/extension.c
src/backend/commands/matview.c
src/backend/commands/portalcmds.c
src/backend/commands/prepare.c
src/backend/commands/trigger.c
src/backend/executor/README
src/backend/executor/execMain.c
src/backend/executor/execParallel.c
src/backend/executor/execPartition.c
src/backend/executor/execUtils.c
src/backend/executor/functions.c
src/backend/executor/spi.c
src/backend/optimizer/plan/planner.c
src/backend/optimizer/plan/setrefs.c
src/backend/tcop/postgres.c
src/backend/tcop/pquery.c
src/backend/utils/cache/plancache.c
src/backend/utils/mmgr/portalmem.c
src/include/commands/explain.h
src/include/commands/trigger.h
src/include/executor/execdesc.h
src/include/executor/executor.h
src/include/nodes/execnodes.h
src/include/nodes/pathnodes.h
src/include/nodes/plannodes.h
src/include/utils/plancache.h
src/include/utils/portal.h

index cd6625020a730f45bf2bd98ceeceab9db52297b9..1f4badb4928409bd891b479ad9524f39e7a9ae9f 100644 (file)
@@ -81,7 +81,7 @@ static ExecutorRun_hook_type prev_ExecutorRun = NULL;
 static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
 static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
 
-static bool explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
+static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
 static void explain_ExecutorRun(QueryDesc *queryDesc,
                                ScanDirection direction,
                                uint64 count);
@@ -261,11 +261,9 @@ _PG_init(void)
 /*
  * ExecutorStart hook: start up logging if needed
  */
-static bool
+static void
 explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
 {
-   bool        plan_valid;
-
    /*
     * At the beginning of each top-level statement, decide whether we'll
     * sample this statement.  If nested-statement explaining is enabled,
@@ -301,13 +299,9 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
    }
 
    if (prev_ExecutorStart)
-       plan_valid = prev_ExecutorStart(queryDesc, eflags);
+       prev_ExecutorStart(queryDesc, eflags);
    else
-       plan_valid = standard_ExecutorStart(queryDesc, eflags);
-
-   /* The plan may have become invalid during standard_ExecutorStart() */
-   if (!plan_valid)
-       return false;
+       standard_ExecutorStart(queryDesc, eflags);
 
    if (auto_explain_enabled())
    {
@@ -325,8 +319,6 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
            MemoryContextSwitchTo(oldcxt);
        }
    }
-
-   return true;
 }
 
 /*
index 9778407cba30bbdec2c4d57d6f9b42d9da6cb9d8..d8fdf42df79354e9aec32130e816c4814304083a 100644 (file)
@@ -335,7 +335,7 @@ static PlannedStmt *pgss_planner(Query *parse,
                                 const char *query_string,
                                 int cursorOptions,
                                 ParamListInfo boundParams);
-static bool pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
+static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
 static void pgss_ExecutorRun(QueryDesc *queryDesc,
                             ScanDirection direction,
                             uint64 count);
@@ -989,19 +989,13 @@ pgss_planner(Query *parse,
 /*
  * ExecutorStart hook: start up tracking if needed
  */
-static bool
+static void
 pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
 {
-   bool        plan_valid;
-
    if (prev_ExecutorStart)
-       plan_valid = prev_ExecutorStart(queryDesc, eflags);
+       prev_ExecutorStart(queryDesc, eflags);
    else
-       plan_valid = standard_ExecutorStart(queryDesc, eflags);
-
-   /* The plan may have become invalid during standard_ExecutorStart() */
-   if (!plan_valid)
-       return false;
+       standard_ExecutorStart(queryDesc, eflags);
 
    /*
     * If query has queryId zero, don't track it.  This prevents double
@@ -1024,8 +1018,6 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
            MemoryContextSwitchTo(oldcxt);
        }
    }
-
-   return true;
 }
 
 /*
index cdf47ac6d2ac74144ad352dd3179126ff13f61b8..143a8b8764701298182cfd68655b63d614bbcc31 100644 (file)
@@ -588,27 +588,6 @@ Improve the locking performance of queries that access many relations (Tomas Von
 
 
 
-
-
-
-
-Avoid the locking of pruned partitions during execution (Amit Langote)
-§
-§
-§
-§
-
-
-