ISTM that some of the portal-related memory context naming is a bit
antiquated and at odds with current terminology. In this patch, I
propose to rename PortalMemory to TopPortalContext and rename
Portal->heap to Portal->portalContext, and then clean up some
surrounding APIs.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From df21079da9d002c6ba80f5a968dfdf3ce630808c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sat, 16 Dec 2017 17:26:26 -0500
Subject: [PATCH 1/2] Update portal-related memory context names and API
Rename PortalMemory to TopPortalContext, to avoid confusion with
PortalContext and align naming with similar top-level memory contexts.
Rename PortalData's "heap" field to portalContext. The "heap" naming
seems quite antiquated and confusing. Also get rid of the
PortalGetHeapMemory() macro and access the field directly, which we do
for other portal fields, so this abstraction doesn't buy anything.
---
src/backend/commands/portalcmds.c | 10 +++++-----
src/backend/commands/prepare.c | 2 +-
src/backend/executor/spi.c | 6 +++---
src/backend/tcop/postgres.c | 2 +-
src/backend/tcop/pquery.c | 16 ++++++++--------
src/backend/utils/mmgr/portalmem.c | 32 ++++++++++++++++----------------
src/include/utils/portal.h | 3 +--
7 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/src/backend/commands/portalcmds.c
b/src/backend/commands/portalcmds.c
index 76d6cf154c..e19b2d82a7 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -96,7 +96,7 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo
params,
*/
portal = CreatePortal(cstmt->portalname, false, false);
- oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ oldContext = MemoryContextSwitchTo(portal->portalContext);
plan = copyObject(plan);
@@ -363,7 +363,7 @@ PersistHoldablePortal(Portal portal)
ActivePortal = portal;
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
- PortalContext = PortalGetHeapMemory(portal);
+ PortalContext = portal->portalContext;
MemoryContextSwitchTo(PortalContext);
@@ -450,10 +450,10 @@ PersistHoldablePortal(Portal portal)
PopActiveSnapshot();
/*
- * We can now release any subsidiary memory of the portal's heap
context;
+ * We can now release any subsidiary memory of the portal's context;
* we'll never use it again. The executor already dropped its context,
- * but this will clean up anything that glommed onto the portal's heap
via
+ * but this will clean up anything that glommed onto the portal's
context via
* PortalContext.
*/
- MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
+ MemoryContextDeleteChildren(portal->portalContext);
}
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index be7222f003..2f57d52b05 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -239,7 +239,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
portal->visible = false;
/* Copy the plan's saved query string into the portal's memory */
- query_string = MemoryContextStrdup(PortalGetHeapMemory(portal),
+ query_string = MemoryContextStrdup(portal->portalContext,
entry->plansource->query_string);
/* Replan if needed, and increment plan refcount for portal */
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index f3da2ddd08..46b8360330 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1183,7 +1183,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr
plan,
}
/* Copy the plan's query string into the portal */
- query_string = MemoryContextStrdup(PortalGetHeapMemory(portal),
+ query_string = MemoryContextStrdup(portal->portalContext,
plansource->query_string);
/*
@@ -1213,7 +1213,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr
plan,
* will result in leaking our refcount on the plan, but it
doesn't
* matter because the plan is unsaved and hence transient
anyway.
*/
- oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ oldcontext = MemoryContextSwitchTo(portal->portalContext);
stmt_list = copyObject(stmt_list);
MemoryContextSwitchTo(oldcontext);
ReleaseCachedPlan(cplan, false);
@@ -1311,7 +1311,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr
plan,
*/
if (paramLI)
{
- oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ oldcontext = MemoryContextSwitchTo(portal->portalContext);
paramLI = copyParamList(paramLI);
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 1ae9ac2d57..704853ceda 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1608,7 +1608,7 @@ exec_bind_message(StringInfo input_message)
* don't want a failure to occur between GetCachedPlan and
* PortalDefineQuery; that would result in leaking our plancache
refcount.
*/
- oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ oldContext = MemoryContextSwitchTo(portal->portalContext);
/* Copy the plan's query string into the portal */
query_string = pstrdup(psrc->query_string);
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 2f98135a59..3d59291f5d 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -466,9 +466,9 @@ PortalStart(Portal portal, ParamListInfo params,
ActivePortal = portal;
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
- PortalContext = PortalGetHeapMemory(portal);
+ PortalContext = portal->portalContext;
- oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ oldContext = MemoryContextSwitchTo(PortalContext);
/* Must remember portal param list, if any */
portal->portalParams = params;
@@ -634,7 +634,7 @@ PortalSetResultFormat(Portal portal, int nFormats, int16
*formats)
return;
natts = portal->tupDesc->natts;
portal->formats = (int16 *)
- MemoryContextAlloc(PortalGetHeapMemory(portal),
+ MemoryContextAlloc(portal->portalContext,
natts * sizeof(int16));
if (nFormats > 1)
{
@@ -748,7 +748,7 @@ PortalRun(Portal portal, long count, bool isTopLevel, bool
run_once,
ActivePortal = portal;
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
- PortalContext = PortalGetHeapMemory(portal);
+ PortalContext = portal->portalContext;
MemoryContextSwitchTo(PortalContext);
@@ -1184,7 +1184,7 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
completionTag);
/* Some utility statements may change context on us */
- MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+ MemoryContextSwitchTo(portal->portalContext);
/*
* Some utility commands may pop the ActiveSnapshot stack from under us,
@@ -1343,9 +1343,9 @@ PortalRunMulti(Portal portal,
/*
* Clear subsidiary contexts to recover temporary memory.
*/
- Assert(PortalGetHeapMemory(portal) == CurrentMemoryContext);
+ Assert(portal->portalContext == CurrentMemoryContext);
- MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
+ MemoryContextDeleteChildren(portal->portalContext);
}
/* Pop the snapshot if we pushed one. */
@@ -1424,7 +1424,7 @@ PortalRunFetch(Portal portal,
ActivePortal = portal;
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
- PortalContext = PortalGetHeapMemory(portal);
+ PortalContext = portal->portalContext;
oldContext = MemoryContextSwitchTo(PortalContext);
diff --git a/src/backend/utils/mmgr/portalmem.c
b/src/backend/utils/mmgr/portalmem.c
index d03b779407..9f5f08af72 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -87,7 +87,7 @@ do { \
elog(WARNING, "trying to delete portal name that does not
exist"); \
} while(0)
-static MemoryContext PortalMemory = NULL;
+static MemoryContext TopPortalContext = NULL;
/* ----------------------------------------------------------------
@@ -104,10 +104,10 @@ EnablePortalManager(void)
{
HASHCTL ctl;
- Assert(PortalMemory == NULL);
+ Assert(TopPortalContext == NULL);
- PortalMemory = AllocSetContextCreate(TopMemoryContext,
-
"PortalMemory",
+ TopPortalContext = AllocSetContextCreate(TopMemoryContext,
+
"TopPortalContext",
ALLOCSET_DEFAULT_SIZES);
ctl.keysize = MAX_PORTALNAME_LEN;
@@ -193,12 +193,12 @@ CreatePortal(const char *name, bool allowDup, bool
dupSilent)
}
/* make new portal structure */
- portal = (Portal) MemoryContextAllocZero(PortalMemory, sizeof *portal);
+ portal = (Portal) MemoryContextAllocZero(TopPortalContext, sizeof
*portal);
- /* initialize portal heap context; typically it won't store much */
- portal->heap = AllocSetContextCreate(PortalMemory,
-
"PortalHeapMemory",
-
ALLOCSET_SMALL_SIZES);
+ /* initialize portal context; typically it won't store much */
+ portal->portalContext = AllocSetContextCreate(TopPortalContext,
+
"PortalContext",
+
ALLOCSET_SMALL_SIZES);
/* create a resource owner for the portal */
portal->resowner = ResourceOwnerCreate(CurTransactionResourceOwner,
@@ -263,7 +263,7 @@ CreateNewPortal(void)
*
* If cplan is NULL, then it is the caller's responsibility to ensure that
* the passed plan trees have adequate lifetime. Typically this is done by
- * copying them into the portal's heap context.
+ * copying them into the portal's context.
*
* The caller is also responsible for ensuring that the passed prepStmtName
* (if not NULL) and sourceText have adequate lifetime.
@@ -331,10 +331,10 @@ PortalCreateHoldStore(Portal portal)
/*
* Create the memory context that is used for storage of the tuple set.
- * Note this is NOT a child of the portal's heap memory.
+ * Note this is NOT a child of the portal's PortalContext.
*/
portal->holdContext =
- AllocSetContextCreate(PortalMemory,
+ AllocSetContextCreate(TopPortalContext,
"PortalHoldContext",
ALLOCSET_DEFAULT_SIZES);
@@ -576,9 +576,9 @@ PortalDrop(Portal portal, bool isTopCommit)
MemoryContextDelete(portal->holdContext);
/* release subsidiary storage */
- MemoryContextDelete(PortalGetHeapMemory(portal));
+ MemoryContextDelete(portal->portalContext);
- /* release portal struct (it's in PortalMemory) */
+ /* release portal struct (it's in TopPortalContext) */
pfree(portal);
}
@@ -806,7 +806,7 @@ AtAbort_Portals(void)
* The cleanup hook was the last thing that might have needed
data
* there.
*/
- MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
+ MemoryContextDeleteChildren(portal->portalContext);
}
}
@@ -1000,7 +1000,7 @@ AtSubAbort_Portals(SubTransactionId mySubid,
* The cleanup hook was the last thing that might have needed
data
* there.
*/
- MemoryContextDeleteChildren(PortalGetHeapMemory(portal));
+ MemoryContextDeleteChildren(portal->portalContext);
}
}
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index cb6f00081d..31f506cf97 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -116,7 +116,7 @@ typedef struct PortalData
/* Bookkeeping data */
const char *name; /* portal's name */
const char *prepStmtName; /* source prepared statement (NULL if
none) */
- MemoryContext heap; /* subsidiary memory for portal
*/
+ MemoryContext portalContext;/* subsidiary memory for portal */
ResourceOwner resowner; /* resources owned by portal */
void (*cleanup) (Portal portal); /* cleanup hook */
@@ -202,7 +202,6 @@ typedef struct PortalData
* Access macros for Portal ... use these in preference to field access.
*/
#define PortalGetQueryDesc(portal) ((portal)->queryDesc)
-#define PortalGetHeapMemory(portal) ((portal)->heap)
/* Prototypes for functions in utils/mmgr/portalmem.c */
base-commit: 38fc54703ea4203a537c58332f697c546eaa4bcf
--
2.15.1
From a09c3f393bd00dcb9e4db6abdb426c1ea633f0a1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Sat, 16 Dec 2017 17:43:41 -0500
Subject: [PATCH 2/2] Remove PortalGetQueryDesc()
After having gotten rid of PortalGetHeapMemory(), there seems little
reason to keep one Portal access macro around that offers no actual
abstraction and isn't consistently used anyway.
---
src/backend/commands/portalcmds.c | 4 ++--
src/backend/executor/execCurrent.c | 2 +-
src/backend/tcop/pquery.c | 4 ++--
src/include/utils/portal.h | 5 -----
4 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/portalcmds.c
b/src/backend/commands/portalcmds.c
index e19b2d82a7..b1aa2cf58c 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -277,7 +277,7 @@ PortalCleanup(Portal portal)
* since other mechanisms will take care of releasing executor
resources,
* and we can't be sure that ExecutorEnd itself wouldn't fail.
*/
- queryDesc = PortalGetQueryDesc(portal);
+ queryDesc = portal->queryDesc;
if (queryDesc)
{
/*
@@ -317,7 +317,7 @@ PortalCleanup(Portal portal)
void
PersistHoldablePortal(Portal portal)
{
- QueryDesc *queryDesc = PortalGetQueryDesc(portal);
+ QueryDesc *queryDesc = portal->queryDesc;
Portal saveActivePortal;
ResourceOwner saveResourceOwner;
MemoryContext savePortalContext;
diff --git a/src/backend/executor/execCurrent.c
b/src/backend/executor/execCurrent.c
index a3e962ee67..d7288e106b 100644
--- a/src/backend/executor/execCurrent.c
+++ b/src/backend/executor/execCurrent.c
@@ -75,7 +75,7 @@ execCurrentOf(CurrentOfExpr *cexpr,
(errcode(ERRCODE_INVALID_CURSOR_STATE),
errmsg("cursor \"%s\" is not a SELECT query",
cursor_name)));
- queryDesc = PortalGetQueryDesc(portal);
+ queryDesc = portal->queryDesc;
if (queryDesc == NULL || queryDesc->estate == NULL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_CURSOR_STATE),
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 3d59291f5d..f27c298353 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -885,7 +885,7 @@ PortalRunSelect(Portal portal,
* NB: queryDesc will be NULL if we are fetching from a held cursor or a
* completed utility query; can't use it in that path.
*/
- queryDesc = PortalGetQueryDesc(portal);
+ queryDesc = portal->queryDesc;
/* Caller messed up if we have neither a ready query nor held data. */
Assert(queryDesc || portal->holdStore);
@@ -1694,7 +1694,7 @@ DoPortalRewind(Portal portal)
}
/* Rewind executor, if active */
- queryDesc = PortalGetQueryDesc(portal);
+ queryDesc = portal->queryDesc;
if (queryDesc)
{
PushActiveSnapshot(queryDesc->snapshot);
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index 31f506cf97..cac2a1b4b8 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -198,11 +198,6 @@ typedef struct PortalData
*/
#define PortalIsValid(p) PointerIsValid(p)
-/*
- * Access macros for Portal ... use these in preference to field access.
- */
-#define PortalGetQueryDesc(portal) ((portal)->queryDesc)
-
/* Prototypes for functions in utils/mmgr/portalmem.c */
extern void EnablePortalManager(void);
--
2.15.1
