Skip to content

Commit 1800410

Browse files
tglsfdctglsfdc
committed
Modify UPDATE/DELETE WHERE CURRENT OF to use the FOR UPDATE infrastructure to
locate the target row, if the cursor was declared with FOR UPDATE or FOR SHARE. This approach is more flexible and reliable than digging through the plan tree; for instance it can cope with join cursors. But we still provide the old code for use with non-FOR-UPDATE cursors. Per gripe from Robert Haas.
1 parent 30f272a commit 1800410

File tree

9 files changed

+246
-70
lines changed

9 files changed

+246
-70
lines changed

‎doc/src/sgml/plpgsql.sgml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.135 2008/10/28 22:02:05 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/plpgsql.sgml,v 1.136 2008/11/16 17:34:28 tgl Exp $ -->
22

33
<chapter id="plpgsql">
44
<title><application>PL/pgSQL</application> - <acronym>SQL</acronym> Procedural Language</title>
@@ -2674,9 +2674,10 @@ DELETE FROM <replaceable>table</replaceable> WHERE CURRENT OF <replaceable>curso
26742674

26752675
<para>
26762676
When a cursor is positioned on a table row, that row can be updated
2677-
or deleted using the cursor to identify the row. Note that this
2678-
only works for simple (non-join, non-grouping) cursor queries.
2679-
For additional information see the
2677+
or deleted using the cursor to identify the row. There are
2678+
restrictions on what the cursor's query can be (in particular,
2679+
no grouping) and it's best to use <literal>FOR UPDATE</> in the
2680+
cursor. For additional information see the
26802681
<xref linkend="sql-declare" endterm="sql-declare-title">
26812682
reference page.
26822683
</para>

‎doc/src/sgml/ref/declare.sgml‎

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/declare.sgml,v 1.44 2008/11/14 10:22:46 petere Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/declare.sgml,v 1.45 2008/11/16 17:34:28 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -213,6 +213,12 @@ DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ INSENSITI
213213
specified, then backward fetches are disallowed in any case.
214214
</para>
215215

216+
<para>
217+
Backward fetches are also disallowed when the query
218+
includes <literal>FOR UPDATE</> or <literal>FOR SHARE</>; therefore
219+
<literal>SCROLL</literal> may not be specified in this case.
220+
</para>
221+
216222
<para>
217223
If the cursor's query includes <literal>FOR UPDATE</> or <literal>FOR
218224
SHARE</>, then returned rows are locked at the time they are first
@@ -221,19 +227,40 @@ DECLARE <replaceable class="parameter">name</replaceable> [ BINARY ] [ INSENSITI
221227
these options.
222228
In addition, the returned rows will be the most up-to-date versions;
223229
therefore these options provide the equivalent of what the SQL standard
224-
calls a <quote>sensitive cursor</>. It is often wise to use <literal>FOR
225-
UPDATE</> if the cursor is intended to be used with <command>UPDATE
226-
... WHERE CURRENT OF</> or <command>DELETE ... WHERE CURRENT OF</>,
227-
since this will prevent other sessions from changing the rows between
228-
the time they are fetched and the time they are updated. Without
229-
<literal>FOR UPDATE</>, a subsequent <literal>WHERE CURRENT OF</> command
230-
will have no effect if the row was changed meanwhile.
230+
calls a <quote>sensitive cursor</>. (Specifying <literal>INSENSITIVE</>
231+
together with <literal>FOR UPDATE</> or <literal>FOR SHARE</> is an error.)
231232
</para>
232233

233-
<para>
234-
<literal>SCROLL</literal> may not be specified when the query
235-
includes <literal>FOR UPDATE</> or <literal>FOR SHARE</>.
236-
</para>
234+
<caution>
235+
<para>
236+
It is generally recommended to use <literal>FOR UPDATE</> if the cursor
237+
is intended to be used with <command>UPDATE ... WHERE CURRENT OF</> or
238+
<command>DELETE ... WHERE CURRENT OF</>. Using <literal>FOR UPDATE</>
239+
prevents other sessions from changing the rows between the time they are
240+
fetched and the time they are updated. Without <literal>FOR UPDATE</>,
241+
a subsequent <literal>WHERE CURRENT OF</> command will have no effect if
242+
the row was changed since the cursor was created.
243+
</para>
244+
245+
<para>
246+
Another reason to use <literal>FOR UPDATE</> is that without it, a
247+
subsequent <literal>WHERE CURRENT OF</> might fail if the cursor query
248+
does not meet the SQL standard's rules for being <quote>simply
249+
updatable</> (in particular, the cursor must reference just one table
250+
and not use grouping or <literal>ORDER BY</>). Cursors
251+
that are not simply updatable might work, or might not, depending on plan
252+
choice details; so in the worst case, an application might work in testing
253+
and then fail in production.
254+
</para>
255+
256+
<para>
257+
The main reason not to use <literal>FOR UPDATE</> with <literal>WHERE
258+
CURRENT OF</> is if you need the cursor to be scrollable, or to be
259+
insensitive to the subsequent updates (that is, continue to show the old
260+
data). If this is a requirement, pay close heed to the caveats shown
261+
above.
262+
</para>
263+
</caution>
237264

238265
<para>
239266
The SQL standard only makes provisions for cursors in embedded

‎doc/src/sgml/ref/delete.sgml‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/delete.sgml,v 1.34 2008/11/14 10:22:46 petere Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/delete.sgml,v 1.35 2008/11/16 17:34:28 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -148,10 +148,13 @@ DELETE FROM [ ONLY ] <replaceable class="PARAMETER">table</replaceable> [ [ AS ]
148148
<para>
149149
The name of the cursor to use in a <literal>WHERE CURRENT OF</>
150150
condition. The row to be deleted is the one most recently fetched
151-
from this cursor. The cursor must be a simple (non-join, non-aggregate)
151+
from this cursor. The cursor must be a non-grouping
152152
query on the <command>DELETE</>'s target table.
153153
Note that <literal>WHERE CURRENT OF</> cannot be
154-
specified together with a Boolean condition.
154+
specified together with a Boolean condition. See
155+
<xref linkend="sql-declare" endterm="sql-declare-title">
156+
for more information about using cursors with
157+
<literal>WHERE CURRENT OF</>.
155158
</para>
156159
</listitem>
157160
</varlistentry>
@@ -244,22 +247,22 @@ DELETE FROM films WHERE kind &lt;&gt; 'Musical';
244247
Clear the table <literal>films</literal>:
245248
<programlisting>
246249
DELETE FROM films;
247-
</programlisting>
250+
</programlisting>
248251
</para>
249252

250253
<para>
251254
Delete completed tasks, returning full details of the deleted rows:
252255
<programlisting>
253256
DELETE FROM tasks WHERE status = 'DONE' RETURNING *;
254-
</programlisting>
257+
</programlisting>
255258
</para>
256259

257260
<para>
258261
Delete the row of <structname>tasks</> on which the cursor
259262
<literal>c_tasks</> is currently positioned:
260263
<programlisting>
261264
DELETE FROM tasks WHERE CURRENT OF c_tasks;
262-
</programlisting>
265+
</programlisting>
263266
</para>
264267
</refsect1>
265268

‎doc/src/sgml/ref/update.sgml‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!--
2-
$PostgreSQL: pgsql/doc/src/sgml/ref/update.sgml,v 1.47 2008/11/14 10:22:47 petere Exp $
2+
$PostgreSQL: pgsql/doc/src/sgml/ref/update.sgml,v 1.48 2008/11/16 17:34:28 tgl Exp $
33
PostgreSQL documentation
44
-->
55

@@ -167,10 +167,13 @@ UPDATE [ ONLY ] <replaceable class="PARAMETER">table</replaceable> [ [ AS ] <rep
167167
<para>
168168
The name of the cursor to use in a <literal>WHERE CURRENT OF</>
169169
condition. The row to be updated is the one most recently fetched
170-
from this cursor. The cursor must be a simple (non-join, non-aggregate)
170+
from this cursor. The cursor must be a non-grouping
171171
query on the <command>UPDATE</>'s target table.
172172
Note that <literal>WHERE CURRENT OF</> cannot be
173-
specified together with a Boolean condition.
173+
specified together with a Boolean condition. See
174+
<xref linkend="sql-declare" endterm="sql-declare-title">
175+
for more information about using cursors with
176+
<literal>WHERE CURRENT OF</>.
174177
</para>
175178
</listitem>
176179
</varlistentry>
@@ -331,7 +334,7 @@ COMMIT;
331334
<literal>c_films</> is currently positioned:
332335
<programlisting>
333336
UPDATE films SET kind = 'Dramatic' WHERE CURRENT OF c_films;
334-
</programlisting>
337+
</programlisting>
335338
</para>
336339
</refsect1>
337340

‎src/backend/executor/execCurrent.c‎

Lines changed: 110 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/backend/executor/execCurrent.c,v 1.7 2008/05/12 00:00:48 alvherre Exp $
9+
* $PostgreSQL: pgsql/src/backend/executor/execCurrent.c,v 1.8 2008/11/16 17:34:28 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -46,10 +46,6 @@ execCurrentOf(CurrentOfExpr *cexpr,
4646
char *table_name;
4747
Portal portal;
4848
QueryDesc *queryDesc;
49-
ScanState *scanstate;
50-
bool lisnull;
51-
Oid tuple_tableoid;
52-
ItemPointer tuple_tid;
5349

5450
/* Get the cursor name --- may have to look up a parameter reference */
5551
if (cexpr->cursor_name)
@@ -79,57 +75,129 @@ execCurrentOf(CurrentOfExpr *cexpr,
7975
errmsg("cursor \"%s\" is not a SELECT query",
8076
cursor_name)));
8177
queryDesc = PortalGetQueryDesc(portal);
82-
if (queryDesc == NULL)
78+
if (queryDesc == NULL || queryDesc->estate == NULL)
8379
ereport(ERROR,
8480
(errcode(ERRCODE_INVALID_CURSOR_STATE),
8581
errmsg("cursor \"%s\" is held from a previous transaction",
8682
cursor_name)));
8783

8884
/*
89-
* Dig through the cursor's plan to find the scan node. Fail if it's not
90-
* there or buried underneath aggregation.
85+
* We have two different strategies depending on whether the cursor uses
86+
* FOR UPDATE/SHARE or not. The reason for supporting both is that the
87+
* FOR UPDATE code is able to identify a target table in many cases where
88+
* the other code can't, while the non-FOR-UPDATE case allows use of WHERE
89+
* CURRENT OF with an insensitive cursor.
9190
*/
92-
scanstate = search_plan_tree(ExecGetActivePlanTree(queryDesc),
93-
table_oid);
94-
if (!scanstate)
95-
ereport(ERROR,
96-
(errcode(ERRCODE_INVALID_CURSOR_STATE),
97-
errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
98-
cursor_name, table_name)));
91+
if (queryDesc->estate->es_rowMarks)
92+
{
93+
ExecRowMark *erm;
94+
ListCell *lc;
9995

100-
/*
101-
* The cursor must have a current result row: per the SQL spec, it's an
102-
* error if not. We test this at the top level, rather than at the scan
103-
* node level, because in inheritance cases any one table scan could
104-
* easily not be on a row. We want to return false, not raise error, if
105-
* the passed-in table OID is for one of the inactive scans.
106-
*/
107-
if (portal->atStart || portal->atEnd)
108-
ereport(ERROR,
109-
(errcode(ERRCODE_INVALID_CURSOR_STATE),
110-
errmsg("cursor \"%s\" is not positioned on a row",
111-
cursor_name)));
96+
/*
97+
* Here, the query must have exactly one FOR UPDATE/SHARE reference to
98+
* the target table, and we dig the ctid info out of that.
99+
*/
100+
erm = NULL;
101+
foreach(lc, queryDesc->estate->es_rowMarks)
102+
{
103+
ExecRowMark *thiserm = (ExecRowMark *) lfirst(lc);
104+
105+
if (RelationGetRelid(thiserm->relation) == table_oid)
106+
{
107+
if (erm)
108+
ereport(ERROR,
109+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
110+
errmsg("cursor \"%s\" has multiple FOR UPDATE/SHARE references to table \"%s\"",
111+
cursor_name, table_name)));
112+
erm = thiserm;
113+
}
114+
}
112115

113-
/* Now OK to return false if we found an inactive scan */
114-
if (TupIsNull(scanstate->ss_ScanTupleSlot))
116+
if (erm == NULL)
117+
ereport(ERROR,
118+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
119+
errmsg("cursor \"%s\" does not have a FOR UPDATE/SHARE reference to table \"%s\"",
120+
cursor_name, table_name)));
121+
122+
/*
123+
* The cursor must have a current result row: per the SQL spec, it's
124+
* an error if not.
125+
*/
126+
if (portal->atStart || portal->atEnd)
127+
ereport(ERROR,
128+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
129+
errmsg("cursor \"%s\" is not positioned on a row",
130+
cursor_name)));
131+
132+
/* Return the currently scanned TID, if there is one */
133+
if (ItemPointerIsValid(&(erm->curCtid)))
134+
{
135+
*current_tid = erm->curCtid;
136+
return true;
137+
}
138+
139+
/*
140+
* This table didn't produce the cursor's current row; some other
141+
* inheritance child of the same parent must have. Signal caller
142+
* to do nothing on this table.
143+
*/
115144
return false;
145+
}
146+
else
147+
{
148+
ScanState *scanstate;
149+
bool lisnull;
150+
Oid tuple_tableoid;
151+
ItemPointer tuple_tid;
152+
153+
/*
154+
* Without FOR UPDATE, we dig through the cursor's plan to find the
155+
* scan node. Fail if it's not there or buried underneath
156+
* aggregation.
157+
*/
158+
scanstate = search_plan_tree(ExecGetActivePlanTree(queryDesc),
159+
table_oid);
160+
if (!scanstate)
161+
ereport(ERROR,
162+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
163+
errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
164+
cursor_name, table_name)));
116165

117-
/* Use slot_getattr to catch any possible mistakes */
118-
tuple_tableoid = DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
119-
TableOidAttributeNumber,
120-
&lisnull));
121-
Assert(!lisnull);
122-
tuple_tid = (ItemPointer)
123-
DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
124-
SelfItemPointerAttributeNumber,
125-
&lisnull));
126-
Assert(!lisnull);
166+
/*
167+
* The cursor must have a current result row: per the SQL spec, it's
168+
* an error if not. We test this at the top level, rather than at the
169+
* scan node level, because in inheritance cases any one table scan
170+
* could easily not be on a row. We want to return false, not raise
171+
* error, if the passed-in table OID is for one of the inactive scans.
172+
*/
173+
if (portal->atStart || portal->atEnd)
174+
ereport(ERROR,
175+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
176+
errmsg("cursor \"%s\" is not positioned on a row",
177+
cursor_name)));
127178

128-
Assert(tuple_tableoid == table_oid);
179+
/* Now OK to return false if we found an inactive scan */
180+
if (TupIsNull(scanstate->ss_ScanTupleSlot))
181+
return false;
129182

130-
*current_tid = *tuple_tid;
183+
/* Use slot_getattr to catch any possible mistakes */
184+
tuple_tableoid =
185+
DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
186+
TableOidAttributeNumber,
187+
&lisnull));
188+
Assert(!lisnull);
189+
tuple_tid = (ItemPointer)
190+
DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
191+
SelfItemPointerAttributeNumber,
192+
&lisnull));
193+
Assert(!lisnull);
131194

132-
return true;
195+
Assert(tuple_tableoid == table_oid);
196+
197+
*current_tid = *tuple_tid;
198+
199+
return true;
200+
}
133201
}
134202

135203
/*

‎src/backend/executor/execMain.c‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.316 2008/11/15 19:43:45 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.317 2008/11/16 17:34:28 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -609,6 +609,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
609609
/* We'll locate the junk attrs below */
610610
erm->ctidAttNo = InvalidAttrNumber;
611611
erm->toidAttNo = InvalidAttrNumber;
612+
ItemPointerSetInvalid(&(erm->curCtid));
612613
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
613614
}
614615

@@ -1418,6 +1419,7 @@ lnext: ;
14181419
if (tableoid != RelationGetRelid(erm->relation))
14191420
{
14201421
/* this child is inactive right now */
1422+
ItemPointerSetInvalid(&(erm->curCtid));
14211423
continue;
14221424
}
14231425
}
@@ -1481,6 +1483,9 @@ lnext: ;
14811483
elog(ERROR, "unrecognized heap_lock_tuple status: %u",
14821484
test);
14831485
}
1486+
1487+
/* Remember tuple TID for WHERE CURRENT OF */
1488+
erm->curCtid = tuple.t_self;
14841489
}
14851490
}
14861491

0 commit comments

Comments
 (0)