Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.92) (envelope-from ) id 1jHXON-0005mc-6s for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Mar 2020 18:38:11 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1jHXOL-000418-Pt for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Mar 2020 18:38:09 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1jHXOL-00040z-4c for pgsql-hackers@lists.postgresql.org; Thu, 26 Mar 2020 18:38:09 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jHXOH-0005AD-2V for pgsql-hackers@postgresql.org; Thu, 26 Mar 2020 18:38:08 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id 02QIbxgC010354; Thu, 26 Mar 2020 14:38:00 -0400 From: Tom Lane To: Andres Freund cc: Pavel Stehule , Amit Langote , PostgreSQL Hackers Subject: Re: plan cache overhead on plpgsql expression In-reply-to: <11118.1585231340@sss.pgh.pa.us> References: <1478.1584815045@sss.pgh.pa.us> <20200325211825.gf2ta424kwkfghbr@alap3.anarazel.de> <15634.1585173110@sss.pgh.pa.us> <20200325224903.pwsx4pccyktq7tx6@alap3.anarazel.de> <11118.1585231340@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Thu, 26 Mar 2020 10:02:20 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <10275.1585247768.0@sss.pgh.pa.us> Date: Thu, 26 Mar 2020 14:37:59 -0400 Message-ID: <10353.1585247879@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <10275.1585247768.1@sss.pgh.pa.us> I wrote: > I had a thought about a possibly-cleaner way to do this. We could invent > a resowner function, say ResourceOwnerReleaseAllPlanCacheRefs, that > explicitly releases all plancache pins it knows about. So plpgsql > would not call the regular ResourceOwnerRelease entry point at all, > but just call that and then ResourceOwnerDelete, again relying on the > assertions therein to catch anything not released. Here's a version that does it like that. This does seem marginally nicer than the other way. I have a feeling that at some point we'll want to expose resowners' contents more generally, but I'm not quite sure what the use-cases will be, so I don't want to design that now. Also, I studied the question of DO blocks' eval_estate + resowner more carefully, and eventually concluded that the way it's being done is okay --- it doesn't leak memory, as I'd first suspected. But it's surely underdocumented, so I added some comments about it. I also concluded as part of that study that it's probably best if we *do* make the resowner parentage different in the two cases after all. So this has the "shared" resowner as a child of TopTransactionResourceOwner after all (which means we don't need to delete it explicitly), while a DO block's private resowner is standalone (so it needs an explicit deletion). Testing that reminded me of the other regression test failure I'd seen when I first tried to do it: select_parallel.sql shows a WARNING about a plancache leak in a parallel worker process. When I looked into the reason for that, it turned out that some cowboy has split XACT_EVENT_COMMIT into XACT_EVENT_COMMIT and XACT_EVENT_PARALLEL_COMMIT (where the latter is used in parallel workers) without bothering to fix the collateral damage to plpgsql. So plpgsql_xact_cb isn't doing any cleanup in parallel workers, and hasn't been for a couple of releases. The bad effects of that are probably limited given that the worker process will exit after committing, but I still think that that part of this patch is a bug fix that needs to be back-patched. (Just looking at what FreeExecutorState does, I wonder whether jit_release_context has any side-effects that are visible outside the process? But I bet I can make a test case that shows issues even without JIT, based on the failure to call ExprContext shutdown callbacks.) Anyway, I think this is committable now. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="0001-add-test-cases-2.patch"; charset="us-ascii" Content-ID: <10275.1585247768.2@sss.pgh.pa.us> Content-Description: 0001-add-test-cases-2.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index 70a9c34..193df8a 100644 --- a/src/pl/plpgsql/src/Makefile +++ b/src/pl/plpgsql/src/Makefile @@ -33,8 +33,8 @@ DATA =3D plpgsql.control plpgsql--1.0.sql REGRESS_OPTS =3D --dbname=3D$(PL_TESTDB) = REGRESS =3D plpgsql_call plpgsql_control plpgsql_copy plpgsql_domain \ - plpgsql_record plpgsql_cache plpgsql_transaction plpgsql_trap \ - plpgsql_trigger plpgsql_varprops + plpgsql_record plpgsql_cache plpgsql_simple plpgsql_transaction \ + plpgsql_trap plpgsql_trigger plpgsql_varprops = # where to find gen_keywordlist.pl and subsidiary files TOOLSDIR =3D $(top_srcdir)/src/tools diff --git a/src/pl/plpgsql/src/expected/plpgsql_simple.out b/src/pl/plpgs= ql/src/expected/plpgsql_simple.out new file mode 100644 index 0000000..5a2fefa --- /dev/null +++ b/src/pl/plpgsql/src/expected/plpgsql_simple.out @@ -0,0 +1,68 @@ +-- +-- Tests for plpgsql's handling of "simple" expressions +-- +-- Check that changes to an inline-able function are handled correctly +create function simplesql(int) returns int language sql +as 'select $1'; +create function simplecaller() returns int language plpgsql +as $$ +declare + sum int :=3D 0; +begin + for n in 1..10 loop + sum :=3D sum + simplesql(n); + if n =3D 5 then + create or replace function simplesql(int) returns int language sql + as 'select $1 + 100'; + end if; + end loop; + return sum; +end$$; +select simplecaller(); + simplecaller = +-------------- + 555 +(1 row) + +-- Check that changes in search path are dealt with correctly +create schema simple1; +create function simple1.simpletarget(int) returns int language plpgsql +as $$begin return $1; end$$; +create function simpletarget(int) returns int language plpgsql +as $$begin return $1 + 100; end$$; +create or replace function simplecaller() returns int language plpgsql +as $$ +declare + sum int :=3D 0; +begin + for n in 1..10 loop + sum :=3D sum + simpletarget(n); + if n =3D 5 then + set local search_path =3D 'simple1'; + end if; + end loop; + return sum; +end$$; +select simplecaller(); + simplecaller = +-------------- + 555 +(1 row) + +-- try it with non-volatile functions, too +alter function simple1.simpletarget(int) immutable; +alter function simpletarget(int) immutable; +select simplecaller(); + simplecaller = +-------------- + 555 +(1 row) + +-- make sure flushing local caches changes nothing +\c - +select simplecaller(); + simplecaller = +-------------- + 555 +(1 row) + diff --git a/src/pl/plpgsql/src/sql/plpgsql_simple.sql b/src/pl/plpgsql/sr= c/sql/plpgsql_simple.sql new file mode 100644 index 0000000..8a95768 --- /dev/null +++ b/src/pl/plpgsql/src/sql/plpgsql_simple.sql @@ -0,0 +1,61 @@ +-- +-- Tests for plpgsql's handling of "simple" expressions +-- + +-- Check that changes to an inline-able function are handled correctly +create function simplesql(int) returns int language sql +as 'select $1'; + +create function simplecaller() returns int language plpgsql +as $$ +declare + sum int :=3D 0; +begin + for n in 1..10 loop + sum :=3D sum + simplesql(n); + if n =3D 5 then + create or replace function simplesql(int) returns int language sql + as 'select $1 + 100'; + end if; + end loop; + return sum; +end$$; + +select simplecaller(); + + +-- Check that changes in search path are dealt with correctly +create schema simple1; + +create function simple1.simpletarget(int) returns int language plpgsql +as $$begin return $1; end$$; + +create function simpletarget(int) returns int language plpgsql +as $$begin return $1 + 100; end$$; + +create or replace function simplecaller() returns int language plpgsql +as $$ +declare + sum int :=3D 0; +begin + for n in 1..10 loop + sum :=3D sum + simpletarget(n); + if n =3D 5 then + set local search_path =3D 'simple1'; + end if; + end loop; + return sum; +end$$; + +select simplecaller(); + +-- try it with non-volatile functions, too +alter function simple1.simpletarget(int) immutable; +alter function simpletarget(int) immutable; + +select simplecaller(); + +-- make sure flushing local caches changes nothing +\c - + +select simplecaller(); ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="0002-faster-revalidation-2.patch"; charset="us-ascii" Content-ID: <10275.1585247768.3@sss.pgh.pa.us> Content-Description: 0002-faster-revalidation-2.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache= /plancache.c index dbae18d..8e27b03 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -1278,6 +1278,160 @@ ReleaseCachedPlan(CachedPlan *plan, bool useResOwn= er) } = /* + * CachedPlanAllowsSimpleValidityCheck: can we use CachedPlanIsSimplyVali= d? + * + * This function, together with CachedPlanIsSimplyValid, provides a fast = path + * for revalidating "simple" generic plans. The core requirement to be s= imple + * is that the plan must not require taking any locks, which translates t= o + * not touching any tables; this happens to match up well with an importa= nt + * use-case in PL/pgSQL. This function tests whether that's true, along + * with checking some other corner cases that we'd rather not bother with + * handling in the fast path. (Note that it's still possible for such a = plan + * to be invalidated, for example due to a change in a function that was + * inlined into the plan.) + * + * This must only be called on known-valid generic plans (eg, ones just + * returned by GetCachedPlan). If it returns true, the caller may re-use + * the cached plan as long as CachedPlanIsSimplyValid returns true; that + * check is much cheaper than the full revalidation done by GetCachedPlan= . + * Nonetheless, no required checks are omitted. + */ +bool +CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource, + CachedPlan *plan) +{ + ListCell *lc; + + /* Sanity-check that the caller gave us a validated generic plan. */ + Assert(plansource->magic =3D=3D CACHEDPLANSOURCE_MAGIC); + Assert(plan->magic =3D=3D CACHEDPLAN_MAGIC); + Assert(plansource->is_valid); + Assert(plan->is_valid); + Assert(plan =3D=3D plansource->gplan); + + /* We don't support oneshot plans here. */ + if (plansource->is_oneshot) + return false; + Assert(!plan->is_oneshot); + + /* + * If the plan is dependent on RLS considerations, or it's transient, + * reject. These things probably can't ever happen for table-free + * queries, but for safety's sake let's check. + */ + if (plansource->dependsOnRLS) + return false; + if (plan->dependsOnRole) + return false; + if (TransactionIdIsValid(plan->saved_xmin)) + return false; + + /* + * Reject if AcquirePlannerLocks would have anything to do. This is + * simplistic, but there's no need to inquire any more carefully; indeed= , + * for current callers it shouldn't even be possible to hit any of these + * checks. + */ + foreach(lc, plansource->query_list) + { + Query *query =3D lfirst_node(Query, lc); + + if (query->commandType =3D=3D CMD_UTILITY) + return false; + if (query->rtable || query->cteList || query->hasSubLinks) + return false; + } + + /* + * Reject if AcquireExecutorLocks would have anything to do. This is + * probably unnecessary given the previous check, but let's be safe. + */ + foreach(lc, plan->stmt_list) + { + PlannedStmt *plannedstmt =3D lfirst_node(PlannedStmt, lc); + ListCell *lc2; + + if (plannedstmt->commandType =3D=3D CMD_UTILITY) + return false; + + /* + * We have to grovel through the rtable because it's likely to contain + * an RTE_RESULT relation, rather than being totally empty. + */ + foreach(lc2, plannedstmt->rtable) + { + RangeTblEntry *rte =3D (RangeTblEntry *) lfirst(lc2); + + if (rte->rtekind =3D=3D RTE_RELATION) + return false; + } + } + + /* + * Okay, it's simple. Note that what we've primarily established here i= s + * that no locks need be taken before checking the plan's is_valid flag. + */ + return true; +} + +/* + * CachedPlanIsSimplyValid: quick check for plan still being valid + * + * This function must not be used unless CachedPlanAllowsSimpleValidityCh= eck + * previously said it was OK. + * + * If the plan is valid, and "owner" is not NULL, record a refcount on + * the plan in that resowner before returning. It is caller's responsibi= lity + * to be sure that a refcount is held on any plan that's being actively u= sed. + * + * The code here is unconditionally safe as long as the only use of this + * CachedPlanSource is in connection with the particular CachedPlan point= er + * that's passed in. If the plansource were being used for other purpose= s, + * it's possible that its generic plan could be invalidated and regenerat= ed + * while the current caller wasn't looking, and then there could be a cha= nce + * collision of address between this caller's now-stale plan pointer and = the + * actual address of the new generic plan. For current uses, that scenar= io + * can't happen; but with a plansource shared across multiple uses, it'd = be + * advisable to also save plan->generation and verify that that still mat= ches. + */ +bool +CachedPlanIsSimplyValid(CachedPlanSource *plansource, CachedPlan *plan, + ResourceOwner owner) +{ + /* + * Careful here: since the caller doesn't necessarily hold a refcount on + * the plan to start with, it's possible that "plan" is a dangling + * pointer. Don't dereference it until we've verified that it still + * matches the plansource's gplan (which is either valid or NULL). + */ + Assert(plansource->magic =3D=3D CACHEDPLANSOURCE_MAGIC); + + /* + * Has cache invalidation fired on this plan? We can check this right + * away since there are no locks that we'd need to acquire first. + */ + if (!plansource->is_valid || plan !=3D plansource->gplan || !plan->is_va= lid) + return false; + + Assert(plan->magic =3D=3D CACHEDPLAN_MAGIC); + + /* Is the search_path still the same as when we made it? */ + Assert(plansource->search_path !=3D NULL); + if (!OverrideSearchPathMatchesCurrent(plansource->search_path)) + return false; + + /* It's still good. Bump refcount if requested. */ + if (owner) + { + ResourceOwnerEnlargePlanCacheRefs(owner); + plan->refcount++; + ResourceOwnerRememberPlanCacheRef(owner, plan); + } + + return true; +} + +/* * CachedPlanSetParentContext: move a CachedPlanSource to a new memory co= ntext * * This can only be applied to unsaved plans; once saved, a plan always diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/res= owner/resowner.c index 3c39e48..8bc2c4e 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -679,6 +679,30 @@ ResourceOwnerReleaseInternal(ResourceOwner owner, } = /* + * ResourceOwnerReleaseAllPlanCacheRefs + * Release the plancache references (only) held by this owner. + * + * We might eventually add similar functions for other resource types, + * but for now, only this is needed. + */ +void +ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner) +{ + ResourceOwner save; + Datum foundres; + + save =3D CurrentResourceOwner; + CurrentResourceOwner =3D owner; + while (ResourceArrayGetAny(&(owner->planrefarr), &foundres)) + { + CachedPlan *res =3D (CachedPlan *) DatumGetPointer(foundres); + + ReleaseCachedPlan(res, true); + } + CurrentResourceOwner =3D save; +} + +/* * ResourceOwnerDelete * Delete an owner object and its descendants. * diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index 6a5953c..037edd0 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -20,6 +20,8 @@ #include "nodes/params.h" #include "tcop/cmdtag.h" #include "utils/queryenvironment.h" +#include "utils/resowner.h" + = /* Forward declaration, to avoid including parsenodes.h here */ struct RawStmt; @@ -220,6 +222,12 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *pl= ansource, QueryEnvironment *queryEnv); extern void ReleaseCachedPlan(CachedPlan *plan, bool useResOwner); = +extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansou= rce, + CachedPlan *plan); +extern bool CachedPlanIsSimplyValid(CachedPlanSource *plansource, + CachedPlan *plan, + ResourceOwner owner); + extern CachedExpression *GetCachedExpression(Node *expr); extern void FreeCachedExpression(CachedExpression *cexpr); = diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index 850bf69..878f39c 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -71,6 +71,7 @@ extern void ResourceOwnerRelease(ResourceOwner owner, ResourceReleasePhase phase, bool isCommit, bool isTopLevel); +extern void ResourceOwnerReleaseAllPlanCacheRefs(ResourceOwner owner); extern void ResourceOwnerDelete(ResourceOwner owner); extern ResourceOwner ResourceOwnerGetParent(ResourceOwner owner); extern void ResourceOwnerNewParent(ResourceOwner owner, diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index d3ad4fa..6f8ae2c 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -84,6 +84,13 @@ typedef struct * has its own simple-expression EState, which is cleaned up at exit from * plpgsql_inline_handler(). DO blocks still use the simple_econtext_sta= ck, * though, so that subxact abort cleanup does the right thing. + * + * (However, if a DO block executes COMMIT or ROLLBACK, then exec_stmt_co= mmit + * or exec_stmt_rollback will unlink it from the DO's simple-expression E= State + * and create a new shared EState that will be used thenceforth. The ori= ginal + * EState will be cleaned up when we get back to plpgsql_inline_handler. = This + * is a bit ugly, but it isn't worth doing better, since scenarios like t= his + * can't result in indefinite accumulation of state trees.) */ typedef struct SimpleEcontextStackEntry { @@ -96,6 +103,15 @@ static EState *shared_simple_eval_estate =3D NULL; static SimpleEcontextStackEntry *simple_econtext_stack =3D NULL; = /* + * In addition to the shared simple-eval EState, we have a shared resourc= e + * owner that holds refcounts on the CachedPlans for any "simple" express= ions + * we have evaluated in the current transaction. This allows us to avoid + * continually grabbing and releasing a plan refcount when a simple expre= ssion + * is used over and over. + */ +static ResourceOwner shared_simple_eval_resowner =3D NULL; + +/* * Memory management within a plpgsql function generally works with three * contexts: * @@ -314,7 +330,8 @@ static int exec_stmt_set(PLpgSQL_execstate *estate, static void plpgsql_estate_setup(PLpgSQL_execstate *estate, PLpgSQL_function *func, ReturnSetInfo *rsi, - EState *simple_eval_estate); + EState *simple_eval_estate, + ResourceOwner simple_eval_resowner); static void exec_eval_cleanup(PLpgSQL_execstate *estate); = static void exec_prepare_plan(PLpgSQL_execstate *estate, @@ -440,16 +457,19 @@ static char *format_preparedparamsdata(PLpgSQL_execs= tate *estate, * * This is also used to execute inline code blocks (DO blocks). The only * difference that this code is aware of is that for a DO block, we want - * to use a private simple_eval_estate, which is created and passed in by - * the caller. For regular functions, pass NULL, which implies using - * shared_simple_eval_estate. (When using a private simple_eval_estate, + * to use a private simple_eval_estate and a private simple_eval_resowner= , + * which are created and passed in by the caller. For regular functions, + * pass NULL, which implies using shared_simple_eval_estate and + * shared_simple_eval_resowner. (When using a private simple_eval_estate= , * we must also use a private cast hashtable, but that's taken care of * within plpgsql_estate_setup.) * ---------- */ Datum plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, - EState *simple_eval_estate, bool atomic) + EState *simple_eval_estate, + ResourceOwner simple_eval_resowner, + bool atomic) { PLpgSQL_execstate estate; ErrorContextCallback plerrcontext; @@ -460,7 +480,7 @@ plpgsql_exec_function(PLpgSQL_function *func, Function= CallInfo fcinfo, * Setup the execution state */ plpgsql_estate_setup(&estate, func, (ReturnSetInfo *) fcinfo->resultinfo= , - simple_eval_estate); + simple_eval_estate, simple_eval_resowner); estate.atomic =3D atomic; = /* @@ -897,7 +917,7 @@ plpgsql_exec_trigger(PLpgSQL_function *func, /* * Setup the execution state */ - plpgsql_estate_setup(&estate, func, NULL, NULL); + plpgsql_estate_setup(&estate, func, NULL, NULL, NULL); estate.trigdata =3D trigdata; = /* @@ -1135,7 +1155,7 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, E= ventTriggerData *trigdata) /* * Setup the execution state */ - plpgsql_estate_setup(&estate, func, NULL, NULL); + plpgsql_estate_setup(&estate, func, NULL, NULL, NULL); estate.evtrigdata =3D trigdata; = /* @@ -2319,6 +2339,7 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_st= mt_call *stmt) * some internal state. */ estate->simple_eval_estate =3D NULL; + estate->simple_eval_resowner =3D NULL; plpgsql_create_econtext(estate); } = @@ -3874,7 +3895,8 @@ static void plpgsql_estate_setup(PLpgSQL_execstate *estate, PLpgSQL_function *func, ReturnSetInfo *rsi, - EState *simple_eval_estate) + EState *simple_eval_estate, + ResourceOwner simple_eval_resowner) { HASHCTL ctl; = @@ -3965,6 +3987,11 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate, estate->cast_hash =3D shared_cast_hash; estate->cast_hash_context =3D shared_cast_context; } + /* likewise for the simple-expression resource owner */ + if (simple_eval_resowner) + estate->simple_eval_resowner =3D simple_eval_resowner; + else + estate->simple_eval_resowner =3D shared_simple_eval_resowner; = /* * We start with no stmt_mcontext; one will be created only if needed. @@ -4825,6 +4852,7 @@ exec_stmt_commit(PLpgSQL_execstate *estate, PLpgSQL_= stmt_commit *stmt) } = estate->simple_eval_estate =3D NULL; + estate->simple_eval_resowner =3D NULL; plpgsql_create_econtext(estate); = return PLPGSQL_RC_OK; @@ -4847,6 +4875,7 @@ exec_stmt_rollback(PLpgSQL_execstate *estate, PLpgSQ= L_stmt_rollback *stmt) } = estate->simple_eval_estate =3D NULL; + estate->simple_eval_resowner =3D NULL; plpgsql_create_econtext(estate); = return PLPGSQL_RC_OK; @@ -6059,8 +6088,6 @@ loop_exit: * someone might redefine a SQL function that had been inlined into the s= imple * expression. That cannot cause a simple expression to become non-simpl= e (or * vice versa), but we do have to handle replacing the expression tree. - * Fortunately it's normally inexpensive to call SPI_plan_get_cached_plan= for - * a simple expression. * * Note: if pass-by-reference, the result is in the eval_mcontext. * It will be freed when exec_eval_cleanup is done. @@ -6076,7 +6103,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, { ExprContext *econtext =3D estate->eval_econtext; LocalTransactionId curlxid =3D MyProc->lxid; - CachedPlan *cplan; + ParamListInfo paramLI; void *save_setup_arg; bool need_snapshot; MemoryContext oldcontext; @@ -6090,29 +6117,92 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, /* * If expression is in use in current xact, don't touch it. */ - if (expr->expr_simple_in_use && expr->expr_simple_lxid =3D=3D curlxid) + if (unlikely(expr->expr_simple_in_use) && + expr->expr_simple_lxid =3D=3D curlxid) return false; = /* - * Revalidate cached plan, so that we will notice if it became stale. (W= e - * need to hold a refcount while using the plan, anyway.) If replanning - * is needed, do that work in the eval_mcontext. + * Check to see if the cached plan has been invalidated. If not, and th= is + * is the first use in the current transaction, save a plan refcount in + * the simple-expression resowner. */ - oldcontext =3D MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan =3D SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); + if (likely(CachedPlanIsSimplyValid(expr->expr_simple_plansource, + expr->expr_simple_plan, + (expr->expr_simple_plan_lxid !=3D curlxid ? + estate->simple_eval_resowner : NULL)))) + { + /* + * It's still good, so just remember that we have a refcount on the + * plan in the current transaction. (If we already had one, this + * assignment is a no-op.) + */ + expr->expr_simple_plan_lxid =3D curlxid; + } + else + { + /* Need to replan */ + CachedPlan *cplan; = - /* - * We can't get a failure here, because the number of CachedPlanSources = in - * the SPI plan can't change from what exec_simple_check_plan saw; it's = a - * property of the raw parsetree generated from the query text. - */ - Assert(cplan !=3D NULL); + /* + * If we have a valid refcount on some previous version of the plan, + * release it, so we don't leak plans intra-transaction. + */ + if (expr->expr_simple_plan_lxid =3D=3D curlxid) + { + ResourceOwner saveResourceOwner =3D CurrentResourceOwner; = - /* If it got replanned, update our copy of the simple expression */ - if (cplan->generation !=3D expr->expr_simple_generation) - { + CurrentResourceOwner =3D estate->simple_eval_resowner; + ReleaseCachedPlan(expr->expr_simple_plan, true); + CurrentResourceOwner =3D saveResourceOwner; + expr->expr_simple_plan =3D NULL; + expr->expr_simple_plan_lxid =3D InvalidLocalTransactionId; + } + + /* Do the replanning work in the eval_mcontext */ + oldcontext =3D MemoryContextSwitchTo(get_eval_mcontext(estate)); + cplan =3D SPI_plan_get_cached_plan(expr->plan); + MemoryContextSwitchTo(oldcontext); + + /* + * We can't get a failure here, because the number of + * CachedPlanSources in the SPI plan can't change from what + * exec_simple_check_plan saw; it's a property of the raw parsetree + * generated from the query text. + */ + Assert(cplan !=3D NULL); + + /* + * These tests probably can't fail either, but if they do, cope by + * declaring the plan to be non-simple. On success, we'll acquire a + * refcount on the new plan, stored in simple_eval_resowner. + */ + if (CachedPlanAllowsSimpleValidityCheck(expr->expr_simple_plansource, + cplan) && + CachedPlanIsSimplyValid(expr->expr_simple_plansource, cplan, + estate->simple_eval_resowner)) + { + /* Remember that we have the refcount */ + expr->expr_simple_plan =3D cplan; + expr->expr_simple_plan_lxid =3D curlxid; + } + else + { + /* Release SPI_plan_get_cached_plan's refcount */ + ReleaseCachedPlan(cplan, true); + /* Mark expression as non-simple, and fail */ + expr->expr_simple_expr =3D NULL; + return false; + } + + /* + * SPI_plan_get_cached_plan acquired a plan refcount stored in the + * active resowner. We don't need that anymore, so release it. + */ + ReleaseCachedPlan(cplan, true); + + /* Extract desired scalar expression from cached plan */ exec_save_simple_expr(expr, cplan); + /* better recheck r/w safety, as it could change due to inlining */ if (expr->rwparam >=3D 0) exec_check_rw_parameter(expr, expr->rwparam); @@ -6128,16 +6218,24 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, * Set up ParamListInfo to pass to executor. For safety, save and resto= re * estate->paramLI->parserSetupArg around our use of the param list. */ - save_setup_arg =3D estate->paramLI->parserSetupArg; + paramLI =3D estate->paramLI; + save_setup_arg =3D paramLI->parserSetupArg; = - econtext->ecxt_param_list_info =3D setup_param_list(estate, expr); + /* + * We can skip using setup_param_list() in favor of just doing this + * unconditionally, because there's no need for the optimization of + * possibly setting ecxt_param_list_info to NULL; we've already forced u= se + * of a generic plan. + */ + paramLI->parserSetupArg =3D (void *) expr; + econtext->ecxt_param_list_info =3D paramLI; = /* * Prepare the expression for execution, if it's not been done already i= n * the current transaction. (This will be forced to happen if we called * exec_save_simple_expr above.) */ - if (expr->expr_simple_lxid !=3D curlxid) + if (unlikely(expr->expr_simple_lxid !=3D curlxid)) { oldcontext =3D MemoryContextSwitchTo(estate->simple_eval_estate->es_que= ry_cxt); expr->expr_simple_state =3D @@ -6185,7 +6283,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, = econtext->ecxt_param_list_info =3D NULL; = - estate->paramLI->parserSetupArg =3D save_setup_arg; + paramLI->parserSetupArg =3D save_setup_arg; = if (need_snapshot) PopActiveSnapshot(); @@ -6193,11 +6291,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, MemoryContextSwitchTo(oldcontext); = /* - * Now we can release our refcount on the cached plan. - */ - ReleaseCachedPlan(cplan, true); - - /* * That's it. */ return true; @@ -7984,10 +8077,35 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, = PLpgSQL_expr *expr) /* Can't fail, because we checked for a single CachedPlanSource above */ Assert(cplan !=3D NULL); = - /* Share the remaining work with replan code path */ - exec_save_simple_expr(expr, cplan); + /* + * Verify that plancache.c thinks the plan is simple enough to use + * CachedPlanIsSimplyValid. Given the restrictions above, it's unlikely + * that this could fail, but if it does, just treat plan as not simple. + */ + if (CachedPlanAllowsSimpleValidityCheck(plansource, cplan)) + { + /* + * OK, use CachedPlanIsSimplyValid to save a refcount on the plan in + * the simple-expression resowner. This shouldn't fail either, but if + * somehow it does, again we can cope by treating plan as not simple. + */ + if (CachedPlanIsSimplyValid(plansource, cplan, + estate->simple_eval_resowner)) + { + /* Remember that we have the refcount */ + expr->expr_simple_plansource =3D plansource; + expr->expr_simple_plan =3D cplan; + expr->expr_simple_plan_lxid =3D MyProc->lxid; + + /* Share the remaining work with the replan code path */ + exec_save_simple_expr(expr, cplan); + } + } = - /* Release our plan refcount */ + /* + * Release the plan refcount obtained by SPI_plan_get_cached_plan. (Thi= s + * refcount is held by the wrong resowner, so we can't just repurpose it= .) + */ ReleaseCachedPlan(cplan, true); } = @@ -8060,7 +8178,6 @@ exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan= *cplan) * current transaction". */ expr->expr_simple_expr =3D tle_expr; - expr->expr_simple_generation =3D cplan->generation; expr->expr_simple_state =3D NULL; expr->expr_simple_in_use =3D false; expr->expr_simple_lxid =3D InvalidLocalTransactionId; @@ -8196,7 +8313,7 @@ exec_set_found(PLpgSQL_execstate *estate, bool state= ) * * We may need to create a new shared_simple_eval_estate too, if there's = not * one already for the current transaction. The EState will be cleaned u= p at - * transaction end. + * transaction end. Ditto for shared_simple_eval_resowner. */ static void plpgsql_create_econtext(PLpgSQL_execstate *estate) @@ -8208,8 +8325,11 @@ plpgsql_create_econtext(PLpgSQL_execstate *estate) * one already in the current transaction. The EState is made a child o= f * TopTransactionContext so it will have the right lifespan. * - * Note that this path is never taken when executing a DO block; the - * required EState was already made by plpgsql_inline_handler. + * Note that this path is never taken when beginning a DO block; the + * required EState was already made by plpgsql_inline_handler. However, + * if the DO block executes COMMIT or ROLLBACK, then we'll come here and + * make a shared EState to use for the rest of the DO block. That's OK; + * see the comments above struct SimpleEcontextStackEntry. */ if (estate->simple_eval_estate =3D=3D NULL) { @@ -8225,6 +8345,18 @@ plpgsql_create_econtext(PLpgSQL_execstate *estate) } = /* + * Likewise for the simple-expression resource owner. + */ + if (estate->simple_eval_resowner =3D=3D NULL) + { + if (shared_simple_eval_resowner =3D=3D NULL) + shared_simple_eval_resowner =3D + ResourceOwnerCreate(TopTransactionResourceOwner, + "PL/pgSQL simple expressions"); + estate->simple_eval_resowner =3D shared_simple_eval_resowner; + } + + /* * Create a child econtext for the current function. */ estate->eval_econtext =3D CreateExprContext(estate->simple_eval_estate); @@ -8270,29 +8402,40 @@ plpgsql_destroy_econtext(PLpgSQL_execstate *estate= ) * plpgsql_xact_cb --- post-transaction-commit-or-abort cleanup * * If a simple-expression EState was created in the current transaction, - * it has to be cleaned up. + * it has to be cleaned up. The same for the simple-expression resowner. */ void plpgsql_xact_cb(XactEvent event, void *arg) { /* - * If we are doing a clean transaction shutdown, free the EState (so tha= t - * any remaining resources will be released correctly). In an abort, we - * expect the regular abort recovery procedures to release everything of - * interest. + * If we are doing a clean transaction shutdown, free the EState and tel= l + * the resowner to release whatever plancache references it has, so that + * all remaining resources will be released correctly. (We don't need t= o + * actually delete the resowner here; deletion of the + * TopTransactionResourceOwner will take care of that.) + * + * In an abort, we expect the regular abort recovery procedures to relea= se + * everything of interest, so just clear our pointers. */ - if (event =3D=3D XACT_EVENT_COMMIT || event =3D=3D XACT_EVENT_PREPARE) + if (event =3D=3D XACT_EVENT_COMMIT || + event =3D=3D XACT_EVENT_PARALLEL_COMMIT || + event =3D=3D XACT_EVENT_PREPARE) { simple_econtext_stack =3D NULL; = if (shared_simple_eval_estate) FreeExecutorState(shared_simple_eval_estate); shared_simple_eval_estate =3D NULL; + if (shared_simple_eval_resowner) + ResourceOwnerReleaseAllPlanCacheRefs(shared_simple_eval_resowner); + shared_simple_eval_resowner =3D NULL; } - else if (event =3D=3D XACT_EVENT_ABORT) + else if (event =3D=3D XACT_EVENT_ABORT || + event =3D=3D XACT_EVENT_PARALLEL_ABORT) { simple_econtext_stack =3D NULL; shared_simple_eval_estate =3D NULL; + shared_simple_eval_resowner =3D NULL; } } = diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handl= er.c index b434818..97cf849 100644 --- a/src/pl/plpgsql/src/pl_handler.c +++ b/src/pl/plpgsql/src/pl_handler.c @@ -262,7 +262,9 @@ plpgsql_call_handler(PG_FUNCTION_ARGS) retval =3D (Datum) 0; } else - retval =3D plpgsql_exec_function(func, fcinfo, NULL, !nonatomic); + retval =3D plpgsql_exec_function(func, fcinfo, + NULL, NULL, + !nonatomic); } PG_FINALLY(); { @@ -297,6 +299,7 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) PLpgSQL_function *func; FmgrInfo flinfo; EState *simple_eval_estate; + ResourceOwner simple_eval_resowner; Datum retval; int rc; = @@ -323,22 +326,33 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) flinfo.fn_oid =3D InvalidOid; flinfo.fn_mcxt =3D CurrentMemoryContext; = - /* Create a private EState for simple-expression execution */ + /* + * Create a private EState and resowner for simple-expression execution. + * Notice that these are NOT tied to transaction-level resources; they + * must survive any COMMIT/ROLLBACK the DO block executes, since we will + * unconditionally try to clean them up below. (Hence, be wary of addin= g + * anything that could fail between here and the PG_TRY block.) + */ simple_eval_estate =3D CreateExecutorState(); + simple_eval_resowner =3D + ResourceOwnerCreate(NULL, "PL/pgSQL DO block simple expressions"); = /* And run the function */ PG_TRY(); { - retval =3D plpgsql_exec_function(func, fake_fcinfo, simple_eval_estate,= codeblock->atomic); + retval =3D plpgsql_exec_function(func, fake_fcinfo, + simple_eval_estate, + simple_eval_resowner, + codeblock->atomic); } PG_CATCH(); { /* * We need to clean up what would otherwise be long-lived resources * accumulated by the failed DO block, principally cached plans for - * statements (which can be flushed with plpgsql_free_function_memory) - * and execution trees for simple expressions, which are in the - * private EState. + * statements (which can be flushed by plpgsql_free_function_memory), + * execution trees for simple expressions, which are in the private + * EState, and cached-plan refcounts held by the private resowner. * * Before releasing the private EState, we must clean up any * simple_econtext_stack entries pointing into it, which we can do by @@ -351,8 +365,10 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) GetCurrentSubTransactionId(), 0, NULL); = - /* Clean up the private EState */ + /* Clean up the private EState and resowner */ FreeExecutorState(simple_eval_estate); + ResourceOwnerReleaseAllPlanCacheRefs(simple_eval_resowner); + ResourceOwnerDelete(simple_eval_resowner); = /* Function should now have no remaining use-counts ... */ func->use_count--; @@ -366,8 +382,10 @@ plpgsql_inline_handler(PG_FUNCTION_ARGS) } PG_END_TRY(); = - /* Clean up the private EState */ + /* Clean up the private EState and resowner */ FreeExecutorState(simple_eval_estate); + ResourceOwnerReleaseAllPlanCacheRefs(simple_eval_resowner); + ResourceOwnerDelete(simple_eval_resowner); = /* Function should now have no remaining use-counts ... */ func->use_count--; diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 69df330..1af2595 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -231,12 +231,21 @@ typedef struct PLpgSQL_expr = /* fields for "simple expression" fast-path execution: */ Expr *expr_simple_expr; /* NULL means not a simple expr */ - int expr_simple_generation; /* plancache generation we checked */ Oid expr_simple_type; /* result type Oid, if simple */ int32 expr_simple_typmod; /* result typmod, if simple */ bool expr_simple_mutable; /* true if simple expr is mutable */ = /* + * If the expression was ever determined to be simple, we remember its + * CachedPlanSource and CachedPlan here. If expr_simple_plan_lxid match= es + * current LXID, then we hold a refcount on expr_simple_plan in the + * current transaction. Otherwise we need to get one before re-using it= . + */ + CachedPlanSource *expr_simple_plansource; /* extracted from "plan" */ + CachedPlan *expr_simple_plan; /* extracted from "plan" */ + LocalTransactionId expr_simple_plan_lxid; + + /* * if expr is simple AND prepared in current transaction, * expr_simple_state and expr_simple_in_use are valid. Test validity by * seeing if expr_simple_lxid matches current LXID. (If not, @@ -1082,8 +1091,9 @@ typedef struct PLpgSQL_execstate */ ParamListInfo paramLI; = - /* EState to use for "simple" expression evaluation */ + /* EState and resowner to use for "simple" expression evaluation */ EState *simple_eval_estate; + ResourceOwner simple_eval_resowner; = /* lookup table to use for executing type casts */ HTAB *cast_hash; @@ -1268,6 +1278,7 @@ extern void _PG_init(void); extern Datum plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo, EState *simple_eval_estate, + ResourceOwner simple_eval_resowner, bool atomic); extern HeapTuple plpgsql_exec_trigger(PLpgSQL_function *func, TriggerData *trigdata); ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="0003-faster-searchpath-2.patch"; charset="us-ascii" Content-ID: <10275.1585247768.4@sss.pgh.pa.us> Content-Description: 0003-faster-searchpath-2.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespa= ce.c index 5ff7824..2ec2301 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -126,6 +126,11 @@ * namespaceUser is the userid the path has been computed for. * * Note: all data pointed to by these List variables is in TopMemoryConte= xt. + * + * activePathGeneration is incremented whenever the effective values of + * activeSearchPath/activeCreationNamespace/activeTempCreationPending cha= nge. + * This can be used to quickly detect whether any change has happened sin= ce + * a previous examination of the search path state. */ = /* These variables define the actually active state: */ @@ -138,6 +143,9 @@ static Oid activeCreationNamespace =3D InvalidOid; /* if true, activeCreationNamespace is wrong, it should be temp namespace= */ static bool activeTempCreationPending =3D false; = +/* current generation counter; make sure this is never zero */ +static uint64 activePathGeneration =3D 1; + /* These variables are the values last derived from namespace_search_path= : */ = static List *baseSearchPath =3D NIL; @@ -3373,6 +3381,7 @@ GetOverrideSearchPath(MemoryContext context) schemas =3D list_delete_first(schemas); } result->schemas =3D schemas; + result->generation =3D activePathGeneration; = MemoryContextSwitchTo(oldcxt); = @@ -3393,12 +3402,18 @@ CopyOverrideSearchPath(OverrideSearchPath *path) result->schemas =3D list_copy(path->schemas); result->addCatalog =3D path->addCatalog; result->addTemp =3D path->addTemp; + result->generation =3D path->generation; = return result; } = /* * OverrideSearchPathMatchesCurrent - does path match current setting? + * + * This is tested over and over in some common code paths, and in the typ= ical + * scenario where the active search path seldom changes, it'll always suc= ceed. + * We make that case fast by keeping a generation counter that is advance= d + * whenever the active search path changes. */ bool OverrideSearchPathMatchesCurrent(OverrideSearchPath *path) @@ -3408,6 +3423,10 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath= *path) = recomputeNamespacePath(); = + /* Quick out if already known equal to active path. */ + if (path->generation =3D=3D activePathGeneration) + return true; + /* We scan down the activeSearchPath to see if it matches the input. */ lc =3D list_head(activeSearchPath); = @@ -3440,6 +3459,13 @@ OverrideSearchPathMatchesCurrent(OverrideSearchPath= *path) } if (lc) return false; + + /* + * Update path->generation so that future tests will return quickly, so + * long as the active search path doesn't change. + */ + path->generation =3D activePathGeneration; + return true; } = @@ -3510,6 +3536,14 @@ PushOverrideSearchPath(OverrideSearchPath *newpath) activeCreationNamespace =3D entry->creationNamespace; activeTempCreationPending =3D false; /* XXX is this OK? */ = + /* + * We always increment activePathGeneration when pushing/popping an + * override path. In current usage, these actions always change the + * effective path state, so there's no value in checking to see if it + * didn't change. + */ + activePathGeneration++; + MemoryContextSwitchTo(oldcxt); } = @@ -3551,6 +3585,9 @@ PopOverrideSearchPath(void) activeCreationNamespace =3D baseCreationNamespace; activeTempCreationPending =3D baseTempCreationPending; } + + /* As above, the generation always increments. */ + activePathGeneration++; } = = @@ -3707,6 +3744,7 @@ recomputeNamespacePath(void) ListCell *l; bool temp_missing; Oid firstNS; + bool pathChanged; MemoryContext oldcxt; = /* Do nothing if an override search spec is active. */ @@ -3814,18 +3852,31 @@ recomputeNamespacePath(void) oidlist =3D lcons_oid(myTempNamespace, oidlist); = /* - * Now that we've successfully built the new list of namespace OIDs, sav= e - * it in permanent storage. + * We want to detect the case where the effective value of the base sear= ch + * path variables didn't change. As long as we're doing so, we can avoi= d + * copying the OID list unncessarily. */ - oldcxt =3D MemoryContextSwitchTo(TopMemoryContext); - newpath =3D list_copy(oidlist); - MemoryContextSwitchTo(oldcxt); + if (baseCreationNamespace =3D=3D firstNS && + baseTempCreationPending =3D=3D temp_missing && + equal(oidlist, baseSearchPath)) + { + pathChanged =3D false; + } + else + { + pathChanged =3D true; + + /* Must save OID list in permanent storage. */ + oldcxt =3D MemoryContextSwitchTo(TopMemoryContext); + newpath =3D list_copy(oidlist); + MemoryContextSwitchTo(oldcxt); = - /* Now safe to assign to state variables. */ - list_free(baseSearchPath); - baseSearchPath =3D newpath; - baseCreationNamespace =3D firstNS; - baseTempCreationPending =3D temp_missing; + /* Now safe to assign to state variables. */ + list_free(baseSearchPath); + baseSearchPath =3D newpath; + baseCreationNamespace =3D firstNS; + baseTempCreationPending =3D temp_missing; + } = /* Mark the path valid. */ baseSearchPathValid =3D true; @@ -3836,6 +3887,16 @@ recomputeNamespacePath(void) activeCreationNamespace =3D baseCreationNamespace; activeTempCreationPending =3D baseTempCreationPending; = + /* + * Bump the generation only if something actually changed. (Notice that + * what we compared to was the old state of the base path variables; so + * this does not deal with the situation where we have just popped an + * override path and restored the prior state of the base path. Instead + * we rely on the override-popping logic to have bumped the generation.) + */ + if (pathChanged) + activePathGeneration++; + /* Clean up. */ pfree(rawname); list_free(namelist); @@ -4054,6 +4115,8 @@ AtEOXact_Namespace(bool isCommit, bool parallel) activeSearchPath =3D baseSearchPath; activeCreationNamespace =3D baseCreationNamespace; activeTempCreationPending =3D baseTempCreationPending; + /* Always bump generation --- see note in recomputeNamespacePath */ + activePathGeneration++; } } = @@ -4109,6 +4172,8 @@ AtEOSubXact_Namespace(bool isCommit, SubTransactionI= d mySubid, overrideStack =3D list_delete_first(overrideStack); list_free(entry->searchPath); pfree(entry); + /* Always bump generation --- see note in recomputeNamespacePath */ + activePathGeneration++; } = /* Activate the next level down. */ @@ -4118,6 +4183,12 @@ AtEOSubXact_Namespace(bool isCommit, SubTransaction= Id mySubid, activeSearchPath =3D entry->searchPath; activeCreationNamespace =3D entry->creationNamespace; activeTempCreationPending =3D false; /* XXX is this OK? */ + + /* + * It's probably unnecessary to bump generation here, but this should + * not be a performance-critical case, so better to be over-cautious. + */ + activePathGeneration++; } else { @@ -4125,6 +4196,12 @@ AtEOSubXact_Namespace(bool isCommit, SubTransaction= Id mySubid, activeSearchPath =3D baseSearchPath; activeCreationNamespace =3D baseCreationNamespace; activeTempCreationPending =3D baseTempCreationPending; + + /* + * If we popped an override stack entry, then we already bumped the + * generation above. If we did not, then the above assignments did + * nothing and we need not bump the generation. + */ } } = @@ -4264,6 +4341,7 @@ InitializeSearchPath(void) activeSearchPath =3D baseSearchPath; activeCreationNamespace =3D baseCreationNamespace; activeTempCreationPending =3D baseTempCreationPending; + activePathGeneration++; /* pro forma */ } else { diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespa= ce.h index 3e3a192..2456c08 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -49,12 +49,17 @@ typedef enum TempNamespaceStatus = /* * Structure for xxxOverrideSearchPath functions + * + * The generation counter is private to namespace.c and shouldn't be touc= hed + * by other code. It can be initialized to zero if necessary (that means + * "not known equal to the current active path"). */ typedef struct OverrideSearchPath { List *schemas; /* OIDs of explicitly named schemas */ bool addCatalog; /* implicitly prepend pg_catalog? */ bool addTemp; /* implicitly prepend temp schema? */ + uint64 generation; /* for quick detection of equality to active */ } OverrideSearchPath; = /* ------- =_aaaaaaaaaa0--