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

Reply via email to