Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1591fcb

Browse files
committed
Revert my bad decision of about a year ago to make PortalDefineQuery
responsible for copying the query string into the new Portal. Such copyingis unnecessary in the common code path through exec_simple_query, and inthis case it can be enormously expensive because the string might containa large number of individual commands; we were copying the entire, longstring for each command, resulting in O(N^2) behavior for N commands.(This is the cause of bug #4079.) A second problem with it is thatPortalDefineQuery really can't risk error, because if it elog's beforehaving set up the Portal, we will leak the plancache refcount that thecaller is trying to hand off to the portal. So go back to the design inwhich the caller is responsible for making sure everything is copied intothe portal if necessary.
1 parentad6bf71 commit1591fcb

File tree

5 files changed

+73
-24
lines changed

5 files changed

+73
-24
lines changed

‎src/backend/commands/portalcmds.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.72 2008/03/26 18:48:59 alvherre Exp $
17+
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.73 2008/04/02 18:31:50 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -77,6 +77,9 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
7777
stmt=copyObject(stmt);
7878
stmt->utilityStmt=NULL;/* make it look like plain SELECT */
7979

80+
if (queryString)/* copy the source text too for safety */
81+
queryString=pstrdup(queryString);
82+
8083
PortalDefineQuery(portal,
8184
NULL,
8285
queryString,

‎src/backend/commands/prepare.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Copyright (c) 2002-2008, PostgreSQL Global Development Group
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.84 2008/03/26 18:48:59 alvherre Exp $
13+
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.85 2008/04/02 18:31:50 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -250,9 +250,13 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString,
250250
plan_list=cplan->stmt_list;
251251
}
252252

253+
/*
254+
* Note: we don't bother to copy the source query string into the portal.
255+
* Any errors it might be useful for will already have been reported.
256+
*/
253257
PortalDefineQuery(portal,
254258
NULL,
255-
entry->plansource->query_string,
259+
NULL,
256260
entry->plansource->commandTag,
257261
plan_list,
258262
cplan);

‎src/backend/executor/spi.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.192 2008/04/01 03:09:30 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.193 2008/04/02 18:31:50 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -918,6 +918,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
918918
CachedPlanSource*plansource;
919919
CachedPlan*cplan;
920920
List*stmt_list;
921+
char*query_string;
921922
ParamListInfoparamLI;
922923
Snapshotsnapshot;
923924
MemoryContextoldcontext;
@@ -968,10 +969,22 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
968969
portal=CreatePortal(name, false, false);
969970
}
970971

972+
/*
973+
* Prepare to copy stuff into the portal's memory context. We do all this
974+
* copying first, because it could possibly fail (out-of-memory) and we
975+
* don't want a failure to occur between RevalidateCachedPlan and
976+
* PortalDefineQuery; that would result in leaking our plancache refcount.
977+
*/
978+
oldcontext=MemoryContextSwitchTo(PortalGetHeapMemory(portal));
979+
980+
/* Copy the plan's query string, if available, into the portal */
981+
query_string=plansource->query_string;
982+
if (query_string)
983+
query_string=pstrdup(query_string);
984+
971985
/* If the plan has parameters, copy them into the portal */
972986
if (plan->nargs>0)
973987
{
974-
oldcontext=MemoryContextSwitchTo(PortalGetHeapMemory(portal));
975988
/* sizeof(ParamListInfoData) includes the first array element */
976989
paramLI= (ParamListInfo)palloc(sizeof(ParamListInfoData)+
977990
(plan->nargs-1)*sizeof(ParamExternData));
@@ -1000,11 +1013,12 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
10001013
paramTypByVal,paramTypLen);
10011014
}
10021015
}
1003-
MemoryContextSwitchTo(oldcontext);
10041016
}
10051017
else
10061018
paramLI=NULL;
10071019

1020+
MemoryContextSwitchTo(oldcontext);
1021+
10081022
if (plan->saved)
10091023
{
10101024
/* Replan if needed, and increment plan refcount for portal */
@@ -1025,7 +1039,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
10251039
*/
10261040
PortalDefineQuery(portal,
10271041
NULL,/* no statement name */
1028-
plansource->query_string,
1042+
query_string,
10291043
plansource->commandTag,
10301044
stmt_list,
10311045
cplan);

‎src/backend/tcop/postgres.c

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.547 2008/03/26 18:48:59 alvherre Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.548 2008/04/02 18:31:50 tgl Exp $
1212
*
1313
* NOTES
1414
* this is the "main" module of the postgres backend and
@@ -933,6 +933,11 @@ exec_simple_query(const char *query_string)
933933
/* Don't display the portal in pg_cursors */
934934
portal->visible= false;
935935

936+
/*
937+
* We don't have to copy anything into the portal, because everything
938+
* we are passsing here is in MessageContext, which will outlive the
939+
* portal anyway.
940+
*/
936941
PortalDefineQuery(portal,
937942
NULL,
938943
query_string,
@@ -1356,8 +1361,11 @@ exec_bind_message(StringInfo input_message)
13561361
CachedPlanSource*psrc;
13571362
CachedPlan*cplan;
13581363
Portalportal;
1364+
char*query_string;
1365+
char*saved_stmt_name;
13591366
ParamListInfoparams;
13601367
List*plan_list;
1368+
MemoryContextoldContext;
13611369
boolsave_log_statement_stats=log_statement_stats;
13621370
charmsec_str[32];
13631371

@@ -1461,16 +1469,32 @@ exec_bind_message(StringInfo input_message)
14611469
else
14621470
portal=CreatePortal(portal_name, false, false);
14631471

1472+
/*
1473+
* Prepare to copy stuff into the portal's memory context. We do all this
1474+
* copying first, because it could possibly fail (out-of-memory) and we
1475+
* don't want a failure to occur between RevalidateCachedPlan and
1476+
* PortalDefineQuery; that would result in leaking our plancache refcount.
1477+
*/
1478+
oldContext=MemoryContextSwitchTo(PortalGetHeapMemory(portal));
1479+
1480+
/* Copy the plan's query string, if available, into the portal */
1481+
query_string=psrc->query_string;
1482+
if (query_string)
1483+
query_string=pstrdup(query_string);
1484+
1485+
/* Likewise make a copy of the statement name, unless it's unnamed */
1486+
if (stmt_name[0])
1487+
saved_stmt_name=pstrdup(stmt_name);
1488+
else
1489+
saved_stmt_name=NULL;
1490+
14641491
/*
14651492
* Fetch parameters, if any, and store in the portal's memory context.
14661493
*/
14671494
if (numParams>0)
14681495
{
1469-
MemoryContextoldContext;
14701496
intparamno;
14711497

1472-
oldContext=MemoryContextSwitchTo(PortalGetHeapMemory(portal));
1473-
14741498
/* sizeof(ParamListInfoData) includes the first array element */
14751499
params= (ParamListInfo)palloc(sizeof(ParamListInfoData)+
14761500
(numParams-1)*sizeof(ParamExternData));
@@ -1595,12 +1619,13 @@ exec_bind_message(StringInfo input_message)
15951619
params->params[paramno].pflags=PARAM_FLAG_CONST;
15961620
params->params[paramno].ptype=ptype;
15971621
}
1598-
1599-
MemoryContextSwitchTo(oldContext);
16001622
}
16011623
else
16021624
params=NULL;
16031625

1626+
/* Done storing stuff in portal's context */
1627+
MemoryContextSwitchTo(oldContext);
1628+
16041629
/* Get the result format codes */
16051630
numRFormats=pq_getmsgint(input_message,2);
16061631
if (numRFormats>0)
@@ -1627,7 +1652,6 @@ exec_bind_message(StringInfo input_message)
16271652
}
16281653
else
16291654
{
1630-
MemoryContextoldContext;
16311655
List*query_list;
16321656

16331657
/*
@@ -1665,8 +1689,8 @@ exec_bind_message(StringInfo input_message)
16651689
* Define portal and start execution.
16661690
*/
16671691
PortalDefineQuery(portal,
1668-
stmt_name[0] ?stmt_name :NULL,
1669-
psrc->query_string,
1692+
saved_stmt_name,
1693+
query_string,
16701694
psrc->commandTag,
16711695
plan_list,
16721696
cplan);

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.108 2008/03/25 22:42:45 tgl Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.109 2008/04/02 18:31:50 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -274,9 +274,7 @@ CreateNewPortal(void)
274274
*
275275
* Notes: commandTag shall be NULL if and only if the original query string
276276
* (before rewriting) was an empty string.Also, the passed commandTag must
277-
* be a pointer to a constant string, since it is not copied. However,
278-
* prepStmtName and sourceText, if provided, are copied into the portal's
279-
* heap context for safekeeping.
277+
* be a pointer to a constant string, since it is not copied.
280278
*
281279
* If cplan is provided, then it is a cached plan containing the stmts,
282280
* and the caller must have done RevalidateCachedPlan(), causing a refcount
@@ -285,6 +283,14 @@ CreateNewPortal(void)
285283
* If cplan is NULL, then it is the caller's responsibility to ensure that
286284
* the passed plan trees have adequate lifetime. Typically this is done by
287285
* copying them into the portal's heap context.
286+
*
287+
* The caller is also responsible for ensuring that the passed prepStmtName
288+
* and sourceText (if not NULL) have adequate lifetime.
289+
*
290+
* NB: this function mustn't do much beyond storing the passed values; in
291+
* particular don't do anything that risks elog(ERROR). If that were to
292+
* happen here before storing the cplan reference, we'd leak the plancache
293+
* refcount that the caller is trying to hand off to us.
288294
*/
289295
void
290296
PortalDefineQuery(Portalportal,
@@ -299,10 +305,8 @@ PortalDefineQuery(Portal portal,
299305

300306
Assert(commandTag!=NULL||stmts==NIL);
301307

302-
portal->prepStmtName=prepStmtName ?
303-
MemoryContextStrdup(PortalGetHeapMemory(portal),prepStmtName) :NULL;
304-
portal->sourceText=sourceText ?
305-
MemoryContextStrdup(PortalGetHeapMemory(portal),sourceText) :NULL;
308+
portal->prepStmtName=prepStmtName;
309+
portal->sourceText=sourceText;
306310
portal->commandTag=commandTag;
307311
portal->stmts=stmts;
308312
portal->cplan=cplan;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp