On Tue, Jan 25, 2005 at 12:32:57PM -0500, Tom Lane wrote:
> So the right fix might involve putting the portal into PORTAL_FAILED
> state rather than just zapping it completely.
Strangely, the code comes up simpler after the fix. Patch attached.
Regression test pass. Additionaly I tried both cases mentioned in this
thread; maybe it's worthy to add tests for them too.
alvherre=# begin;
BEGIN
alvherre=# savepoint x;
SAVEPOINT
alvherre=# create table abc (a int);
CREATE TABLE
alvherre=# insert into abc values (5);
INSERT 33616 1
alvherre=# declare foo cursor for select * from abc;
DECLARE CURSOR
alvherre=# rollback to x;
ROLLBACK
alvherre=# fetch from foo; -- hits an Assert()
ERROR: no existe el cursor �foo�
alvherre=# commit;
ROLLBACK
alvherre=# begin;
BEGIN
alvherre=# declare c cursor for select 1 union all select 2;
DECLARE CURSOR
alvherre=# savepoint x;
SAVEPOINT
alvherre=# fetch from c;
?column?
----------
1
(1 fila)
alvherre=# rollback to x;
ROLLBACK
alvherre=# fetch from c;
?column?
----------
2
(1 fila)
alvherre=# commit;
COMMIT
--
Alvaro Herrera (<[EMAIL PROTECTED]>)
"The ability to monopolize a planet is insignificant
next to the power of the source"
Index: portalmem.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/utils/mmgr/portalmem.c,v
retrieving revision 1.76
diff -c -r1.76 portalmem.c
*** portalmem.c 31 Dec 2004 22:02:48 -0000 1.76
--- portalmem.c 25 Jan 2005 17:52:52 -0000
***************
*** 623,663 ****
continue;
/*
! * Force any active portals of my own transaction into FAILED
* state. This is mostly to ensure that a portal running a FETCH
* will go FAILED if the underlying cursor fails. (Note we do
NOT
* want to do this to upper-level portals, since they may be
able
* to continue.)
*/
! if (portal->status == PORTAL_ACTIVE)
portal->status = PORTAL_FAILED;
! /*
! * If the portal is READY then allow it to survive into the
parent
! * transaction; otherwise shut it down.
! */
! if (portal->status == PORTAL_READY)
! {
! portal->createSubid = parentSubid;
! if (portal->resowner)
! ResourceOwnerNewParent(portal->resowner,
parentXactOwner);
! }
! else
{
! /* let portalcmds.c clean up the state it knows about */
! if (PointerIsValid(portal->cleanup))
! {
! (*portal->cleanup) (portal);
! portal->cleanup = NULL;
! }
!
! /*
! * Any resources belonging to the portal will be
released in
! * the upcoming transaction-wide cleanup; they will be
gone
! * before we run PortalDrop.
! */
! portal->resowner = NULL;
}
}
}
--- 623,650 ----
continue;
/*
! * Force any active and ready portals of my own transaction
into FAILED
* state. This is mostly to ensure that a portal running a FETCH
* will go FAILED if the underlying cursor fails. (Note we do
NOT
* want to do this to upper-level portals, since they may be
able
* to continue.)
*/
! if (portal->status == PORTAL_ACTIVE || portal->status ==
PORTAL_READY)
portal->status = PORTAL_FAILED;
! /* let portalcmds.c clean up the state it knows about */
! if (PointerIsValid(portal->cleanup))
{
! (*portal->cleanup) (portal);
! portal->cleanup = NULL;
}
+
+ /*
+ * Any resources belonging to the portal will be released in
+ * the upcoming transaction-wide cleanup; they will be gone
+ * before we run PortalDrop.
+ */
+ portal->resowner = NULL;
}
}
---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings
