Обсуждение: "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"
I found the following behavior surprising. Is there a reason for it? This is 8.3.5. ...Robert rhaas=# BEGIN WORK; BEGIN rhaas=# CREATE TABLE test_table (id serial, foo integer, bar integer); NOTICE: CREATE TABLE will create implicit sequence "test_table_id_seq" for serial column "test_table.id" CREATE TABLE rhaas=# INSERT INTO test_table VALUES (DEFAULT, 1, 1); INSERT 0 1 rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR UPDATE; DECLARE CURSOR rhaas=# FETCH c;id ---- 1 (1 row) rhaas=# UPDATE test_table SET bar = NULL WHERE CURRENT OF c; ERROR: cursor "c" is not a simply updatable scan of table "test_table" rhaas=# \q
"Robert Haas" <robertmhaas@gmail.com> writes: > I found the following behavior surprising. Is there a reason for it? > This is 8.3.5. ...Robert > > rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR UPDATE; > DECLARE CURSOR Skimming the code we don't support WHERE CURRENT OF on a query which involves a Sort above the table specified. The way CURRENT OF works depends on the nature of the plan and depends on there being an active scan of the specified table. Since sort reads all its inputs in advance that information is lost by the time the results are output. If you had an index it would work. That this is tied to the nature of the plan does seem kind of unfortunate. I'm not sure the alternatives are very appealing though -- they would involve a lot of code to track a lot more information for queries that might never need it. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!
"Robert Haas" <robertmhaas@gmail.com> writes:
> I found the following behavior surprising. Is there a reason for it?
Yes.
regards, tom lane
(Oh, you wanted to know what the reason is? It's because a sort step
is not going to pass through any tuple identity information.)
Gregory Stark <stark@enterprisedb.com> writes:
> "Robert Haas" <robertmhaas@gmail.com> writes:
>> I found the following behavior surprising. Is there a reason for it?
>> This is 8.3.5. ...Robert
>>
>> rhaas=# DECLARE c CURSOR FOR SELECT id FROM test_table ORDER BY foo FOR UPDATE;
> Skimming the code we don't support WHERE CURRENT OF on a query which involves
> a Sort above the table specified. The way CURRENT OF works depends on the
> nature of the plan and depends on there being an active scan of the specified
> table. Since sort reads all its inputs in advance that information is lost by
> the time the results are output.
[ dept of second thoughts... ] Actually, given that he said FOR UPDATE,
the plan should be propagating the tuple identity through to top level
so that execMain can do its thing. So in principle we could probably
get the information without widespread changes. This would fit
reasonably well with the spec's requirements too --- any but trivial
cursors are not deemed updatable unless you say FOR UPDATE. But it
would mean two completely independent implementations within
execCurrent.c...
regards, tom lane
> [ dept of second thoughts... ] Actually, given that he said FOR UPDATE, > the plan should be propagating the tuple identity through to top level > so that execMain can do its thing. So in principle we could probably > get the information without widespread changes. This would fit > reasonably well with the spec's requirements too --- any but trivial > cursors are not deemed updatable unless you say FOR UPDATE. But it > would mean two completely independent implementations within > execCurrent.c... What's particularly unfortunate is Greg's comment that this would work if the planner chose an index scan. Had I defined an index on the columns in question, my code might have worked and been deployed to production - and then broken if the planner changed its mind and decided to use a seqscan after all. ISTM any cursor that's going to be updated should specify WHERE UPDATE in the query, but maybe that's not a hard requirement as of now. ...Robert
I wrote:
> [ dept of second thoughts... ] Actually, given that he said FOR UPDATE,
> the plan should be propagating the tuple identity through to top level
> so that execMain can do its thing. So in principle we could probably
> get the information without widespread changes. This would fit
> reasonably well with the spec's requirements too --- any but trivial
> cursors are not deemed updatable unless you say FOR UPDATE. But it
> would mean two completely independent implementations within
> execCurrent.c...
Here's a draft patch (no docs, no regression test) for that. It doesn't
look as ugly as I expected. Comments? I'm hesitant to call this a bug
fix, and we are past feature freeze ...
regards, tom lane
Index: src/backend/executor/execCurrent.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execCurrent.c,v
retrieving revision 1.7
diff -c -r1.7 execCurrent.c
*** src/backend/executor/execCurrent.c 12 May 2008 00:00:48 -0000 1.7
--- src/backend/executor/execCurrent.c 15 Nov 2008 00:04:17 -0000
***************
*** 46,51 ****
--- 46,53 ----
char *table_name;
Portal portal;
QueryDesc *queryDesc;
+ ExecRowMark *erm;
+ ListCell *lc;
ScanState *scanstate;
bool lisnull;
Oid tuple_tableoid;
***************
*** 86,91 ****
--- 88,140 ----
cursor_name)));
/*
+ * If the query uses FOR UPDATE, look through the executor's state for that
+ * to see if we can identify the target row that way. We succeed if there
+ * is exactly one FOR UPDATE item for the requested table. (Note:
+ * presently, FOR UPDATE is not allowed on inheritance trees, so there is
+ * no need to worry about whether a FOR UPDATE item is currently valid.)
+ */
+ erm = NULL;
+ foreach(lc, queryDesc->estate->es_rowMarks)
+ {
+ ExecRowMark *thiserm = (ExecRowMark *) lfirst(lc);
+
+ if (RelationGetRelid(thiserm->relation) == table_oid)
+ {
+ if (erm)
+ {
+ /* multiple references to desired relation */
+ erm = NULL;
+ break;
+ }
+ erm = thiserm;
+ }
+ }
+
+ if (erm)
+ {
+ /*
+ * Okay, we were able to identify the target FOR UPDATE item.
+ *
+ * The cursor must have a current result row: per the SQL spec, it's
+ * an error if not.
+ */
+ if (portal->atStart || portal->atEnd)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_CURSOR_STATE),
+ errmsg("cursor \"%s\" is not positioned on a row",
+ cursor_name)));
+ /* Return the currently scanned TID */
+ if (ItemPointerIsValid(&(erm->curCtid)))
+ {
+ *current_tid = erm->curCtid;
+ return true;
+ }
+ /* Inactive scan? Probably can't happen at the moment */
+ return false;
+ }
+
+ /*
* Dig through the cursor's plan to find the scan node. Fail if it's not
* there or buried underneath aggregation.
*/
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.315
diff -c -r1.315 execMain.c
*** src/backend/executor/execMain.c 6 Nov 2008 20:51:14 -0000 1.315
--- src/backend/executor/execMain.c 15 Nov 2008 00:04:17 -0000
***************
*** 602,607 ****
--- 602,608 ----
erm->noWait = rc->noWait;
/* We'll set up ctidAttno below */
erm->ctidAttNo = InvalidAttrNumber;
+ ItemPointerSetInvalid(&(erm->curCtid));
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
}
***************
*** 1442,1447 ****
--- 1443,1451 ----
elog(ERROR, "unrecognized heap_lock_tuple status: %u",
test);
}
+
+ /* Remember tuple TID for WHERE CURRENT OF */
+ erm->curCtid = tuple.t_self;
}
}
Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.194
diff -c -r1.194 execnodes.h
*** src/include/nodes/execnodes.h 31 Oct 2008 19:37:56 -0000 1.194
--- src/include/nodes/execnodes.h 15 Nov 2008 00:04:17 -0000
***************
*** 376,381 ****
--- 376,382 ----
bool forUpdate; /* true = FOR UPDATE, false = FOR SHARE */
bool noWait; /* NOWAIT option */
AttrNumber ctidAttNo; /* resno of its ctid junk attribute */
+ ItemPointerData curCtid; /* ctid of currently locked tuple */
} ExecRowMark;
"Robert Haas" <robertmhaas@gmail.com> writes:
> What's particularly unfortunate is Greg's comment that this would work
> if the planner chose an index scan. Had I defined an index on the
> columns in question, my code might have worked and been deployed to
> production - and then broken if the planner changed its mind and
> decided to use a seqscan after all.
> ISTM any cursor that's going to be updated should specify WHERE UPDATE
> in the query, but maybe that's not a hard requirement as of now.
Well, it's contrary to SQL spec, which says that sufficiently simple
cursors should be updatable even if they don't say FOR UPDATE.
However ... the more I think about it, the more I wonder if we shouldn't
rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF
*only* for cursors that say FOR UPDATE. Aside from the above issue,
there's an already known and documented risk if you omit FOR UPDATE,
which is that your WHERE CURRENT OF update silently becomes a no-op
if someone else has already updated the target row since your query
started. It seems like not using FOR UPDATE is sufficiently dangerous
practice that requiring it wouldn't be doing our users a disservice.
There is one thing we lack in order to go that far, though: the current
implementation of WHERE CURRENT OF can cope with inheritance queries,
that is you can sayDECLARE c CURSOR ... FROM parent_table ...UPDATE parent_table ... WHERE CURRENT OF c
and the right things will happen even if the cursor is currently showing
a row from some child table. SELECT FOR UPDATE does not presently
support inheritance, so we'd really have to fix that in order to not
have a loss of functionality. This is something that was on my private
to-do list for 8.4 but I hadn't thought of an easy way to do it. But,
revisiting the issue just now, I have an idea: when a FOR UPDATE target
is an inheritance tree, make the plan return not only ctid as a junk
column, but also tableoid. The executor top level could easily use that
to determine which table to actually try to do heap_lock_tuple in.
I haven't looked at the planner code for this lately, but I'm thinking
it is probably less than a day's work to make it happen.
Comments?
regards, tom lane
> Here's a draft patch (no docs, no regression test) for that. It doesn't > look as ugly as I expected. Comments? I'm hesitant to call this a bug > fix, and we are past feature freeze ... Considering the number of and invasiveness of the patches remaining in the queue, I'm inclined to consider this a drop in the bucket, but then I'm biased since I just got bitten by it. ...Robert
> However ... the more I think about it, the more I wonder if we shouldn't > rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF > *only* for cursors that say FOR UPDATE. Aside from the above issue, > there's an already known and documented risk if you omit FOR UPDATE, > which is that your WHERE CURRENT OF update silently becomes a no-op > if someone else has already updated the target row since your query > started. It seems like not using FOR UPDATE is sufficiently dangerous > practice that requiring it wouldn't be doing our users a disservice. Yes, I was reading that documentation today and scratching my head. It didn't quite dawn on me that the solution was to just always use FOR UPDATE, but certainly if it is then you may as well enforce it. I've encountered that no-op behavior in other situations in the past, specifically BEFORE triggers, and it's really weird and confusing, because you typically have to be doing something fairly complicated before the problem case arises, and then it mysteriously fails to work. ...Robert
Tom Lane <tgl@sss.pgh.pa.us> writes: > "Robert Haas" <robertmhaas@gmail.com> writes: >> What's particularly unfortunate is Greg's comment that this would work >> if the planner chose an index scan. Had I defined an index on the >> columns in question, my code might have worked and been deployed to >> production - and then broken if the planner changed its mind and >> decided to use a seqscan after all. > >> ISTM any cursor that's going to be updated should specify WHERE UPDATE >> in the query, but maybe that's not a hard requirement as of now. > > Well, it's contrary to SQL spec, which says that sufficiently simple > cursors should be updatable even if they don't say FOR UPDATE. > > However ... the more I think about it, the more I wonder if we shouldn't > rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF > *only* for cursors that say FOR UPDATE. It is tempting since it would make application code which looped updating WHERE CURRENT OF semantically equivalent to UPDATE FROM. It seems better to have one set of functionality rather than two similar but subtly different sets of functionality available depending on the coding style. > Aside from the above issue, there's an already known and documented risk if > you omit FOR UPDATE, which is that your WHERE CURRENT OF update silently > becomes a no-op if someone else has already updated the target row since > your query started. It seems like not using FOR UPDATE is sufficiently > dangerous practice that requiring it wouldn't be doing our users a > disservice. Could we implicitly add FOR UPDATE when planning and executing a cursor of a sufficiently simple query? How simple does the spec say the query needs to be? > There is one thing we lack in order to go that far, though: the current > implementation of WHERE CURRENT OF can cope with inheritance queries, How would this implementation relate to the issues described in inheritance_planner (which always seemed strange): * inheritance_planner* Generate a plan in the case where the result relation is an* inheritance set.** We haveto handle this case differently from cases where a source relation* is an inheritance set. Source inheritance is expandedat the bottom of the* plan tree (see allpaths.c), but target inheritance has to be expanded at* the top. The reasonis that for UPDATE, each target relation needs a* different targetlist matching its own column set. Also, for bothUPDATE* and DELETE, the executor needs the Append plan node at the top, else it* can't keep track of which table is thecurrent target table. Fortunately,* the UPDATE/DELETE target can never be the nullable side of an outer join,* so it'sOK to generate the plan this way. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support!
Gregory Stark <stark@enterprisedb.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Aside from the above issue, there's an already known and documented risk if
>> you omit FOR UPDATE, which is that your WHERE CURRENT OF update silently
>> becomes a no-op if someone else has already updated the target row since
>> your query started. It seems like not using FOR UPDATE is sufficiently
>> dangerous practice that requiring it wouldn't be doing our users a
>> disservice.
> Could we implicitly add FOR UPDATE when planning and executing a cursor of a
> sufficiently simple query?
No, not unless you want plain SELECTs to suddenly start blocking each
other.
>> There is one thing we lack in order to go that far, though: the current
>> implementation of WHERE CURRENT OF can cope with inheritance queries,
> How would this implementation relate to the issues described in
> inheritance_planner (which always seemed strange):
Yeah, it is very tempting to think about getting rid of all the
inherited-target cruft (both in the planner, and in the executor's weird
interactions between nodeAppend and execMain) in favor of using a
tableoid junk column to figure out which target rel to update.
However there's one other nasty problem to fix, which is that in an
inherited UPDATE you may need a different update targetlist for each
target relation. I'm not seeing a solution for that yet in the context
of this simplified approach.
regards, tom lane
Gregory Stark <stark@enterprisedb.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Well, it's contrary to SQL spec, which says that sufficiently simple
>> cursors should be updatable even if they don't say FOR UPDATE.
>>
>> However ... the more I think about it, the more I wonder if we shouldn't
>> rip out the current search-the-plan-tree hack and allow WHERE CURRENT OF
>> *only* for cursors that say FOR UPDATE.
> It is tempting since it would make application code which looped updating
> WHERE CURRENT OF semantically equivalent to UPDATE FROM. It seems better to
> have one set of functionality rather than two similar but subtly different
> sets of functionality available depending on the coding style.
After playing around with this for awhile I realized that there really
would be a functional loss if we remove support for WHERE CURRENT OF
with non-FOR-UPDATE cursors. Namely, that a non-FOR-UPDATE cursor is
insensitive to subsequent updates, which sometimes is handy. There are
examples of both cases in the portals.sql regression test.
So what I now propose is:
1. If the cursor includes FOR UPDATE/FOR SHARE, use the ExecRowMark
technique to get the target row identity. In this path we fail if there
is not exactly one FOR UPDATE reference to the UPDATE's target table.
(An interesting property here is that you can update from a self-join,
if you mark only one arm of the join as FOR UPDATE. See example in
attached regression test additions.)
2. If the cursor doesn't have FOR UPDATE/SHARE, use the existing code.
In this path we fail if the plan is "too complicated". However, it
shouldn't fail for any case that is simply updatable according to the
SQL spec.
We should revise the documentation to make it very clear that FOR UPDATE
is the preferred way, but that sometimes you might need the other.
Attached is a draft patch that is code-complete but lacks documentation.
The patch is against CVS HEAD, ie, it assumes the SELECT FOR UPDATE
inheritance fixes I made earlier today.
regards, tom lane
Index: src/backend/executor/execCurrent.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execCurrent.c,v
retrieving revision 1.7
diff -c -r1.7 execCurrent.c
*** src/backend/executor/execCurrent.c 12 May 2008 00:00:48 -0000 1.7
--- src/backend/executor/execCurrent.c 16 Nov 2008 00:10:32 -0000
***************
*** 46,55 ****
char *table_name;
Portal portal;
QueryDesc *queryDesc;
- ScanState *scanstate;
- bool lisnull;
- Oid tuple_tableoid;
- ItemPointer tuple_tid;
/* Get the cursor name --- may have to look up a parameter reference */
if (cexpr->cursor_name)
--- 46,51 ----
***************
*** 79,135 ****
errmsg("cursor \"%s\" is not a SELECT query",
cursor_name)));
queryDesc = PortalGetQueryDesc(portal);
! if (queryDesc == NULL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_CURSOR_STATE),
errmsg("cursor \"%s\" is held from a previous transaction",
cursor_name)));
/*
! * Dig through the cursor's plan to find the scan node. Fail if it's not
! * there or buried underneath aggregation.
*/
! scanstate = search_plan_tree(ExecGetActivePlanTree(queryDesc),
! table_oid);
! if (!scanstate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
! cursor_name, table_name)));
! /*
! * The cursor must have a current result row: per the SQL spec, it's an
! * error if not. We test this at the top level, rather than at the scan
! * node level, because in inheritance cases any one table scan could
! * easily not be on a row. We want to return false, not raise error, if
! * the passed-in table OID is for one of the inactive scans.
! */
! if (portal->atStart || portal->atEnd)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not positioned on a row",
! cursor_name)));
! /* Now OK to return false if we found an inactive scan */
! if (TupIsNull(scanstate->ss_ScanTupleSlot))
! return false;
! /* Use slot_getattr to catch any possible mistakes */
! tuple_tableoid = DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
! TableOidAttributeNumber,
! &lisnull));
! Assert(!lisnull);
! tuple_tid = (ItemPointer)
! DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
! SelfItemPointerAttributeNumber,
! &lisnull));
! Assert(!lisnull);
! Assert(tuple_tableoid == table_oid);
! *current_tid = *tuple_tid;
! return true;
}
/*
--- 75,203 ----
errmsg("cursor \"%s\" is not a SELECT query",
cursor_name)));
queryDesc = PortalGetQueryDesc(portal);
! if (queryDesc == NULL || queryDesc->estate == NULL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_CURSOR_STATE),
errmsg("cursor \"%s\" is held from a previous transaction",
cursor_name)));
/*
! * We have two different strategies depending on whether the cursor uses
! * FOR UPDATE/SHARE or not. The reason for supporting both is that the
! * FOR UPDATE code is able to identify a target table in many cases where
! * the other code can't, while the non-FOR-UPDATE case allows use of WHERE
! * CURRENT OF with an insensitive cursor.
*/
! if (queryDesc->estate->es_rowMarks)
! {
! ExecRowMark *erm;
! ListCell *lc;
! /*
! * Here, the query must have exactly one FOR UPDATE/SHARE reference to
! * the target table, and we dig the ctid info out of that.
! */
! erm = NULL;
! foreach(lc, queryDesc->estate->es_rowMarks)
! {
! ExecRowMark *thiserm = (ExecRowMark *) lfirst(lc);
! if (RelationGetRelid(thiserm->relation) == table_oid)
! {
! if (erm)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" has multiple FOR UPDATE/SHARE references to table \"%s\"",
! cursor_name, table_name)));
! erm = thiserm;
! }
! }
! if (erm == NULL)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" does not have a FOR UPDATE/SHARE reference to table \"%s\"",
! cursor_name, table_name)));
!
! /*
! * The cursor must have a current result row: per the SQL spec, it's
! * an error if not.
! */
! if (portal->atStart || portal->atEnd)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not positioned on a row",
! cursor_name)));
! /* Return the currently scanned TID, if there is one */
! if (ItemPointerIsValid(&(erm->curCtid)))
! {
! *current_tid = erm->curCtid;
! return true;
! }
! /*
! * This table didn't produce the cursor's current row; some other
! * inheritance child of the same parent must have. Signal caller
! * to do nothing on this table.
! */
! return false;
! }
! else
! {
! ScanState *scanstate;
! bool lisnull;
! Oid tuple_tableoid;
! ItemPointer tuple_tid;
!
! /*
! * Without FOR UPDATE, we dig through the cursor's plan to find the
! * scan node. Fail if it's not there or buried underneath
! * aggregation.
! */
! scanstate = search_plan_tree(ExecGetActivePlanTree(queryDesc),
! table_oid);
! if (!scanstate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
! cursor_name, table_name)));
!
! /*
! * The cursor must have a current result row: per the SQL spec, it's
! * an error if not. We test this at the top level, rather than at the
! * scan node level, because in inheritance cases any one table scan
! * could easily not be on a row. We want to return false, not raise
! * error, if the passed-in table OID is for one of the inactive scans.
! */
! if (portal->atStart || portal->atEnd)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not positioned on a row",
! cursor_name)));
!
! /* Now OK to return false if we found an inactive scan */
! if (TupIsNull(scanstate->ss_ScanTupleSlot))
! return false;
!
! /* Use slot_getattr to catch any possible mistakes */
! tuple_tableoid =
! DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
! TableOidAttributeNumber,
! &lisnull));
! Assert(!lisnull);
! tuple_tid = (ItemPointer)
! DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
! SelfItemPointerAttributeNumber,
! &lisnull));
! Assert(!lisnull);
!
! Assert(tuple_tableoid == table_oid);
! *current_tid = *tuple_tid;
!
! return true;
! }
}
/*
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.316
diff -c -r1.316 execMain.c
*** src/backend/executor/execMain.c 15 Nov 2008 19:43:45 -0000 1.316
--- src/backend/executor/execMain.c 16 Nov 2008 00:10:32 -0000
***************
*** 609,614 ****
--- 609,615 ----
/* We'll locate the junk attrs below */
erm->ctidAttNo = InvalidAttrNumber;
erm->toidAttNo = InvalidAttrNumber;
+ ItemPointerSetInvalid(&(erm->curCtid));
estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
}
***************
*** 1418,1423 ****
--- 1419,1425 ----
if (tableoid != RelationGetRelid(erm->relation))
{
/* this child is inactive right now */
+ ItemPointerSetInvalid(&(erm->curCtid));
continue;
}
}
***************
*** 1481,1486 ****
--- 1483,1491 ----
elog(ERROR, "unrecognized heap_lock_tuple status: %u",
test);
}
+
+ /* Remember tuple TID for WHERE CURRENT OF */
+ erm->curCtid = tuple.t_self;
}
}
Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.195
diff -c -r1.195 execnodes.h
*** src/include/nodes/execnodes.h 15 Nov 2008 19:43:46 -0000 1.195
--- src/include/nodes/execnodes.h 16 Nov 2008 00:10:32 -0000
***************
*** 381,386 ****
--- 381,387 ----
bool noWait; /* NOWAIT option */
AttrNumber ctidAttNo; /* resno of its ctid junk attribute */
AttrNumber toidAttNo; /* resno of tableoid junk attribute, if any */
+ ItemPointerData curCtid; /* ctid of currently locked tuple, if any */
} ExecRowMark;
Index: src/test/regress/expected/portals.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/portals.out,v
retrieving revision 1.18
diff -c -r1.18 portals.out
*** src/test/regress/expected/portals.out 15 Nov 2008 19:43:47 -0000 1.18
--- src/test/regress/expected/portals.out 16 Nov 2008 00:10:32 -0000
***************
*** 1154,1159 ****
--- 1154,1200 ----
110 | hundred
(3 rows)
+ -- Can update from a self-join, but only if FOR UPDATE says which to use
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5;
+ FETCH 1 FROM c1;
+ f1 | f2 | f1 | f2
+ ----+-----+----+-------
+ 18 | one | 13 | three
+ (1 row)
+
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- fail
+ ERROR: cursor "c1" is not a simply updatable scan of table "uctest"
+ ROLLBACK;
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5 FOR UPDATE;
+ FETCH 1 FROM c1;
+ f1 | f2 | f1 | f2
+ ----+-----+----+-------
+ 18 | one | 13 | three
+ (1 row)
+
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- fail
+ ERROR: cursor "c1" has multiple FOR UPDATE/SHARE references to table "uctest"
+ ROLLBACK;
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5 FOR SHARE OF a;
+ FETCH 1 FROM c1;
+ f1 | f2 | f1 | f2
+ ----+-----+----+-------
+ 18 | one | 13 | three
+ (1 row)
+
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1;
+ SELECT * FROM uctest;
+ f1 | f2
+ -----+---------
+ 13 | three
+ 28 | one
+ 110 | hundred
+ (3 rows)
+
+ ROLLBACK;
-- Check various error cases
DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no such cursor
ERROR: cursor "c1" does not exist
***************
*** 1166,1171 ****
--- 1207,1217 ----
ERROR: cursor "c" is not a simply updatable scan of table "uctest"
ROLLBACK;
BEGIN;
+ DECLARE c CURSOR FOR SELECT * FROM tenk2 FOR SHARE;
+ DELETE FROM uctest WHERE CURRENT OF c; -- fail, cursor on wrong table
+ ERROR: cursor "c" does not have a FOR UPDATE/SHARE reference to table "uctest"
+ ROLLBACK;
+ BEGIN;
DECLARE c CURSOR FOR SELECT * FROM tenk1 JOIN tenk2 USING (unique1);
DELETE FROM tenk1 WHERE CURRENT OF c; -- fail, cursor is on a join
ERROR: cursor "c" is not a simply updatable scan of table "tenk1"
Index: src/test/regress/sql/portals.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/portals.sql,v
retrieving revision 1.15
diff -c -r1.15 portals.sql
*** src/test/regress/sql/portals.sql 15 Nov 2008 19:43:47 -0000 1.15
--- src/test/regress/sql/portals.sql 16 Nov 2008 00:10:32 -0000
***************
*** 404,409 ****
--- 404,427 ----
COMMIT;
SELECT * FROM uctest;
+ -- Can update from a self-join, but only if FOR UPDATE says which to use
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5;
+ FETCH 1 FROM c1;
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- fail
+ ROLLBACK;
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5 FOR UPDATE;
+ FETCH 1 FROM c1;
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- fail
+ ROLLBACK;
+ BEGIN;
+ DECLARE c1 CURSOR FOR SELECT * FROM uctest a, uctest b WHERE a.f1 = b.f1 + 5 FOR SHARE OF a;
+ FETCH 1 FROM c1;
+ UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1;
+ SELECT * FROM uctest;
+ ROLLBACK;
+
-- Check various error cases
DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no such cursor
***************
*** 414,419 ****
--- 432,441 ----
DELETE FROM uctest WHERE CURRENT OF c; -- fail, cursor on wrong table
ROLLBACK;
BEGIN;
+ DECLARE c CURSOR FOR SELECT * FROM tenk2 FOR SHARE;
+ DELETE FROM uctest WHERE CURRENT OF c; -- fail, cursor on wrong table
+ ROLLBACK;
+ BEGIN;
DECLARE c CURSOR FOR SELECT * FROM tenk1 JOIN tenk2 USING (unique1);
DELETE FROM tenk1 WHERE CURRENT OF c; -- fail, cursor is on a join
ROLLBACK;