Skip to content

Commit 65b2f93

Browse files
tglsfdctglsfdc
committed
Fix oversight in initial implementation of PORTAL_ONE_RETURNING mode: we
cannot assume that there's exactly one Query in the Portal, as we can for ONE_SELECT mode, because non-SELECT queries might have extra queries added during rule rewrites. Fix things up so that we'll use ONE_RETURNING mode when a Portal contains one primary (canSetTag) query and that query has a RETURNING list. This appears to be a second showstopper reason for running the Portal to completion before we start to hand anything back --- we want to be sure that the rule-added queries get run too.
1 parent 3d1e01c commit 65b2f93

File tree

5 files changed

+137
-63
lines changed

5 files changed

+137
-63
lines changed

‎src/backend/commands/prepare.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Copyright (c) 2002-2006, PostgreSQL Global Development Group
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.60 2006/08/12 02:52:04 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.61 2006/08/14 22:57:15 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -448,7 +448,7 @@ FetchPreparedStatementResultDesc(PreparedStatement *stmt)
448448
return ExecCleanTypeFromTL(query->targetList, false);
449449

450450
case PORTAL_ONE_RETURNING:
451-
query = (Query *) linitial(stmt->query_list);
451+
query = PortalListGetPrimaryQuery(stmt->query_list);
452452
return ExecCleanTypeFromTL(query->returningList, false);
453453

454454
case PORTAL_UTIL_SELECT:
@@ -505,7 +505,7 @@ FetchPreparedStatementTargetList(PreparedStatement *stmt)
505505
if (strategy == PORTAL_ONE_SELECT)
506506
return ((Query *) linitial(stmt->query_list))->targetList;
507507
if (strategy == PORTAL_ONE_RETURNING)
508-
return ((Query *) linitial(stmt->query_list))->returningList;
508+
return (PortalListGetPrimaryQuery(stmt->query_list))->returningList;
509509
if (strategy == PORTAL_UTIL_SELECT)
510510
{
511511
Node *utilityStmt;

‎src/backend/executor/spi.c

Lines changed: 49 additions & 38 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.156 2006/08/14 13:40:18 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.157 2006/08/14 22:57:15 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -818,31 +818,44 @@ SPI_cursor_open(const char *name, void *plan,
818818
bool read_only)
819819
{
820820
_SPI_plan *spiplan = (_SPI_plan *) plan;
821-
List *qtlist = spiplan->qtlist;
822-
List *ptlist = spiplan->ptlist;
823-
Query *queryTree;
824-
Plan *planTree;
821+
List *qtlist;
822+
List *ptlist;
825823
ParamListInfo paramLI;
826824
Snapshot snapshot;
827825
MemoryContext oldcontext;
828826
Portal portal;
829827
int k;
830828

831-
/* Ensure that the plan contains only one query */
832-
if (list_length(ptlist) != 1 || list_length(qtlist) != 1)
833-
ereport(ERROR,
834-
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
835-
errmsg("cannot open multi-query plan as cursor")));
836-
queryTree = (Query *) linitial((List *) linitial(qtlist));
837-
planTree = (Plan *) linitial(ptlist);
838-
839-
/* Must be a query that returns tuples */
840-
if (!QueryReturnsTuples(queryTree))
829+
/*
830+
* Check that the plan is something the Portal code will special-case
831+
* as returning one tupleset.
832+
*/
833+
if (!SPI_is_cursor_plan(spiplan))
834+
{
835+
/* try to give a good error message */
836+
Query *queryTree;
837+
838+
if (list_length(spiplan->qtlist) != 1)
839+
ereport(ERROR,
840+
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
841+
errmsg("cannot open multi-query plan as cursor")));
842+
queryTree = PortalListGetPrimaryQuery((List *) linitial(spiplan->qtlist));
843+
if (queryTree == NULL)
844+
ereport(ERROR,
845+
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
846+
errmsg("cannot open empty query as cursor")));
841847
ereport(ERROR,
842848
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
843849
/* translator: %s is name of a SQL command, eg INSERT */
844850
errmsg("cannot open %s query as cursor",
845851
CreateQueryTag(queryTree))));
852+
}
853+
854+
Assert(list_length(spiplan->qtlist) == 1);
855+
qtlist = (List *) linitial(spiplan->qtlist);
856+
ptlist = spiplan->ptlist;
857+
if (list_length(qtlist) != list_length(ptlist))
858+
elog(ERROR, "corrupted SPI plan lists");
846859

847860
/* Reset SPI result (note we deliberately don't touch lastoid) */
848861
SPI_processed = 0;
@@ -862,10 +875,10 @@ SPI_cursor_open(const char *name, void *plan,
862875
portal = CreatePortal(name, false, false);
863876
}
864877

865-
/* Switch to portal's memory and copy the parsetree and plan to there */
878+
/* Switch to portal's memory and copy the parsetrees and plans to there */
866879
oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
867-
queryTree = copyObject(queryTree);
868-
planTree = copyObject(planTree);
880+
qtlist = copyObject(qtlist);
881+
ptlist = copyObject(ptlist);
869882

870883
/* If the plan has parameters, set them up */
871884
if (spiplan->nargs > 0)
@@ -907,9 +920,9 @@ SPI_cursor_open(const char *name, void *plan,
907920
PortalDefineQuery(portal,
908921
NULL, /* no statement name */
909922
spiplan->query,
910-
CreateQueryTag(queryTree),
911-
list_make1(queryTree),
912-
list_make1(planTree),
923+
CreateQueryTag(PortalListGetPrimaryQuery(qtlist)),
924+
qtlist,
925+
ptlist,
913926
PortalGetHeapMemory(portal));
914927

915928
MemoryContextSwitchTo(oldcontext);
@@ -918,7 +931,8 @@ SPI_cursor_open(const char *name, void *plan,
918931
* Set up options for portal.
919932
*/
920933
portal->cursorOptions &= ~(CURSOR_OPT_SCROLL | CURSOR_OPT_NO_SCROLL);
921-
if (planTree == NULL || ExecSupportsBackwardScan(planTree))
934+
if (list_length(ptlist) == 1 &&
935+
ExecSupportsBackwardScan((Plan *) linitial(ptlist)))
922936
portal->cursorOptions |= CURSOR_OPT_SCROLL;
923937
else
924938
portal->cursorOptions |= CURSOR_OPT_NO_SCROLL;
@@ -940,16 +954,7 @@ SPI_cursor_open(const char *name, void *plan,
940954
*/
941955
PortalStart(portal, paramLI, snapshot);
942956

943-
/*
944-
* If this test fails then we're out of sync with pquery.c about
945-
* which queries can return tuples...
946-
*/
947-
if (portal->strategy == PORTAL_MULTI_QUERY)
948-
ereport(ERROR,
949-
(errcode(ERRCODE_INVALID_CURSOR_DEFINITION),
950-
/* translator: %s is name of a SQL command, eg INSERT */
951-
errmsg("cannot open %s query as cursor",
952-
CreateQueryTag(queryTree))));
957+
Assert(portal->strategy != PORTAL_MULTI_QUERY);
953958

954959
/* Return the created portal */
955960
return portal;
@@ -1050,21 +1055,27 @@ bool
10501055
SPI_is_cursor_plan(void *plan)
10511056
{
10521057
_SPI_plan *spiplan = (_SPI_plan *) plan;
1053-
List *qtlist;
10541058

10551059
if (spiplan == NULL)
10561060
{
10571061
SPI_result = SPI_ERROR_ARGUMENT;
10581062
return false;
10591063
}
10601064

1061-
qtlist = spiplan->qtlist;
1062-
if (list_length(spiplan->ptlist) == 1 && list_length(qtlist) == 1)
1063-
{
1064-
Query *queryTree = (Query *) linitial((List *) linitial(qtlist));
1065+
if (list_length(spiplan->qtlist) != 1)
1066+
return false; /* not exactly 1 pre-rewrite command */
10651067

1066-
if (QueryReturnsTuples(queryTree))
1068+
switch (ChoosePortalStrategy((List *) linitial(spiplan->qtlist)))
1069+
{
1070+
case PORTAL_ONE_SELECT:
1071+
case PORTAL_ONE_RETURNING:
1072+
case PORTAL_UTIL_SELECT:
1073+
/* OK */
10671074
return true;
1075+
1076+
case PORTAL_MULTI_QUERY:
1077+
/* will not return tuples */
1078+
break;
10681079
}
10691080
return false;
10701081
}

‎src/backend/tcop/pquery.c

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.106 2006/08/12 02:52:05 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.107 2006/08/14 22:57:15 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -213,30 +213,59 @@ ProcessQuery(Query *parsetree,
213213
PortalStrategy
214214
ChoosePortalStrategy(List *parseTrees)
215215
{
216-
PortalStrategy strategy;
217-
218-
strategy = PORTAL_MULTI_QUERY; /* default assumption */
216+
int nSetTag;
217+
ListCell *lc;
219218

219+
/*
220+
* PORTAL_ONE_SELECT and PORTAL_UTIL_SELECT need only consider the
221+
* single-Query-struct case, since there are no rewrite rules that
222+
* can add auxiliary queries to a SELECT or a utility command.
223+
*/
220224
if (list_length(parseTrees) == 1)
221225
{
222226
Query *query = (Query *) linitial(parseTrees);
223227

228+
Assert(IsA(query, Query));
224229
if (query->canSetTag)
225230
{
226231
if (query->commandType == CMD_SELECT &&
227232
query->into == NULL)
228-
strategy = PORTAL_ONE_SELECT;
229-
else if (query->returningList != NIL)
230-
strategy = PORTAL_ONE_RETURNING;
231-
else if (query->commandType == CMD_UTILITY &&
232-
query->utilityStmt != NULL)
233+
return PORTAL_ONE_SELECT;
234+
if (query->commandType == CMD_UTILITY &&
235+
query->utilityStmt != NULL)
233236
{
234237
if (UtilityReturnsTuples(query->utilityStmt))
235-
strategy = PORTAL_UTIL_SELECT;
238+
return PORTAL_UTIL_SELECT;
239+
/* it can't be ONE_RETURNING, so give up */
240+
return PORTAL_MULTI_QUERY;
236241
}
237242
}
238243
}
239-
return strategy;
244+
245+
/*
246+
* PORTAL_ONE_RETURNING has to allow auxiliary queries added by rewrite.
247+
* Choose PORTAL_ONE_RETURNING if there is exactly one canSetTag query
248+
* and it has a RETURNING list.
249+
*/
250+
nSetTag = 0;
251+
foreach(lc, parseTrees)
252+
{
253+
Query *query = (Query *) lfirst(lc);
254+
255+
Assert(IsA(query, Query));
256+
if (query->canSetTag)
257+
{
258+
if (++nSetTag > 1)
259+
return PORTAL_MULTI_QUERY; /* no need to look further */
260+
if (query->returningList == NIL)
261+
return PORTAL_MULTI_QUERY; /* no need to look further */
262+
}
263+
}
264+
if (nSetTag == 1)
265+
return PORTAL_ONE_RETURNING;
266+
267+
/* Else, it's the general case... */
268+
return PORTAL_MULTI_QUERY;
240269
}
241270

242271
/*
@@ -255,7 +284,7 @@ FetchPortalTargetList(Portal portal)
255284
if (portal->strategy == PORTAL_ONE_SELECT)
256285
return ((Query *) linitial(portal->parseTrees))->targetList;
257286
if (portal->strategy == PORTAL_ONE_RETURNING)
258-
return ((Query *) linitial(portal->parseTrees))->returningList;
287+
return (PortalGetPrimaryQuery(portal))->returningList;
259288
if (portal->strategy == PORTAL_UTIL_SELECT)
260289
{
261290
Node *utilityStmt;
@@ -422,7 +451,7 @@ PortalStart(Portal portal, ParamListInfo params, Snapshot snapshot)
422451
* the portal. We do need to set up the result tupdesc.
423452
*/
424453
portal->tupDesc =
425-
ExecCleanTypeFromTL(((Query *) linitial(portal->parseTrees))->returningList, false);
454+
ExecCleanTypeFromTL((PortalGetPrimaryQuery(portal))->returningList, false);
426455

427456
/*
428457
* Reset cursor position data to "start of query"
@@ -894,10 +923,11 @@ FillPortalStore(Portal portal)
894923
{
895924
case PORTAL_ONE_RETURNING:
896925
/*
897-
* We run the query just as if it were in a MULTI portal,
898-
* but send the output to the tuplestore.
926+
* Run the portal to completion just as for the default MULTI_QUERY
927+
* case, but send the primary query's output to the tuplestore.
928+
* Auxiliary query outputs are discarded.
899929
*/
900-
PortalRunMulti(portal, treceiver, treceiver, completionTag);
930+
PortalRunMulti(portal, treceiver, None_Receiver, completionTag);
901931
/* Override default completion tag with actual command result */
902932
portal->commandTag = pstrdup(completionTag);
903933
break;

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

Lines changed: 29 additions & 1 deletion
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.91 2006/08/08 01:23:15 momjian Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.92 2006/08/14 22:57:15 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -146,6 +146,34 @@ GetPortalByName(const char *name)
146146
return portal;
147147
}
148148

149+
/*
150+
* PortalListGetPrimaryQuery
151+
* Get the "primary" Query within a portal, ie, the one marked canSetTag.
152+
*
153+
* Returns NULL if no such Query. If multiple Query structs within the
154+
* portal are marked canSetTag, returns the first one. Neither of these
155+
* cases should occur in present usages of this function.
156+
*
157+
* Note: the reason this is just handed a List is so that prepared statements
158+
* can share the code. For use with a portal, use PortalGetPrimaryQuery
159+
* rather than calling this directly.
160+
*/
161+
Query *
162+
PortalListGetPrimaryQuery(List *parseTrees)
163+
{
164+
ListCell *lc;
165+
166+
foreach(lc, parseTrees)
167+
{
168+
Query *query = (Query *) lfirst(lc);
169+
170+
Assert(IsA(query, Query));
171+
if (query->canSetTag)
172+
return query;
173+
}
174+
return NULL;
175+
}
176+
149177
/*
150178
* CreatePortal
151179
* Returns a new portal given a name.

‎src/include/utils/portal.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
4040
* Portions Copyright (c) 1994, Regents of the University of California
4141
*
42-
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.65 2006/08/12 02:52:06 tgl Exp $
42+
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.66 2006/08/14 22:57:15 tgl Exp $
4343
*
4444
*-------------------------------------------------------------------------
4545
*/
@@ -63,10 +63,13 @@
6363
* tuplestore for access after transaction completion).
6464
*
6565
* PORTAL_ONE_RETURNING: the portal contains a single INSERT/UPDATE/DELETE
66-
* query with a RETURNING clause. On first execution, we run the statement
67-
* and dump its results into the portal tuplestore; the results are then
68-
* returned to the client as demanded. (We can't support suspension of
69-
* the query partway through, because the AFTER TRIGGER code can't cope.)
66+
* query with a RETURNING clause (plus possibly auxiliary queries added by
67+
* rule rewriting). On first execution, we run the portal to completion
68+
* and dump the primary query's results into the portal tuplestore; the
69+
* results are then returned to the client as demanded. (We can't support
70+
* suspension of the query partway through, because the AFTER TRIGGER code
71+
* can't cope, and also because we don't want to risk failing to execute
72+
* all the auxiliary queries.)
7073
*
7174
* PORTAL_UTIL_SELECT: the portal contains a utility statement that returns
7275
* a SELECT-like result (for example, EXPLAIN or SHOW). On first execution,
@@ -187,6 +190,7 @@ typedef struct PortalData
187190
*/
188191
#define PortalGetQueryDesc(portal) ((portal)->queryDesc)
189192
#define PortalGetHeapMemory(portal) ((portal)->heap)
193+
#define PortalGetPrimaryQuery(portal) PortalListGetPrimaryQuery((portal)->parseTrees)
190194

191195

192196
/* Prototypes for functions in utils/mmgr/portalmem.c */
@@ -215,6 +219,7 @@ extern void PortalDefineQuery(Portal portal,
215219
List *parseTrees,
216220
List *planTrees,
217221
MemoryContext queryContext);
222+
extern Query *PortalListGetPrimaryQuery(List *parseTrees);
218223
extern void PortalCreateHoldStore(Portal portal);
219224

220225
#endif /* PORTAL_H */

0 commit comments

Comments
 (0)