Re: [GENERAL] Postgres 7.2 - Updating rows in cursor problem

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Vaclav Kulakovsky <vaclav(dot)kulakovsky(at)definity(dot)cz>
Cc: pgsql-patches(at)postgreSQL(dot)org
Subject: Re: [GENERAL] Postgres 7.2 - Updating rows in cursor problem
Date: 2002-02-14 02:12:05
Message-ID: 5882.1013652725@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers pgsql-patches

Vaclav Kulakovsky <vaclav(dot)kulakovsky(at)definity(dot)cz> writes:
> I've a problem in PG 7.2. If you update rows which are included in plpgsql
> RECORD , updated rows are again added to the RECORD, so you will get into
> infinite loop.

The attached patch against 7.2 appears to fix this problem, as well as
the cursor misbehavior that I exhibited in my followup on pghackers.

If I don't hear any complaints I will commit this soon.

regards, tom lane

*** src/backend/commands/command.c.orig Thu Jan 3 18:19:30 2002
--- src/backend/commands/command.c Wed Feb 13 19:24:00 2002
***************
*** 103,108 ****
--- 103,109 ----
QueryDesc *queryDesc;
EState *estate;
MemoryContext oldcontext;
+ CommandId savedId;
bool temp_desc = false;

/*
***************
*** 156,162 ****
}

/*
! * tell the destination to prepare to receive some tuples.
*/
BeginCommand(name,
queryDesc->operation,
--- 157,163 ----
}

/*
! * Tell the destination to prepare to receive some tuples.
*/
BeginCommand(name,
queryDesc->operation,
***************
*** 169,174 ****
--- 170,183 ----
queryDesc->dest);

/*
+ * Restore the scanCommandId that was current when the cursor was
+ * opened. This ensures that we see the same tuples throughout the
+ * execution of the cursor.
+ */
+ savedId = GetScanCommandId();
+ SetScanCommandId(PortalGetCommandId(portal));
+
+ /*
* Determine which direction to go in, and check to see if we're
* already at the end of the available tuples in that direction. If
* so, do nothing. (This check exists because not all plan node types
***************
*** 213,218 ****
--- 222,232 ----
portal->atStart = true; /* we retrieved 'em all */
}
}
+
+ /*
+ * Restore outer command ID.
+ */
+ SetScanCommandId(savedId);

/*
* Clean up and switch back to old context.
*** src/backend/executor/spi.c.orig Thu Jan 3 15:30:47 2002
--- src/backend/executor/spi.c Wed Feb 13 19:23:55 2002
***************
*** 740,748 ****
_SPI_current->processed = 0;
_SPI_current->tuptable = NULL;

- /* Make up a portal name if none given */
if (name == NULL)
{
for (;;)
{
unnamed_portal_count++;
--- 740,748 ----
_SPI_current->processed = 0;
_SPI_current->tuptable = NULL;

if (name == NULL)
{
+ /* Make up a portal name if none given */
for (;;)
{
unnamed_portal_count++;
***************
*** 755,765 ****

name = portalname;
}
!
! /* Ensure the portal doesn't exist already */
! portal = GetPortalByName(name);
! if (portal != NULL)
! elog(ERROR, "cursor \"%s\" already in use", name);

/* Create the portal */
portal = CreatePortal(name);
--- 755,767 ----

name = portalname;
}
! else
! {
! /* Ensure the portal doesn't exist already */
! portal = GetPortalByName(name);
! if (portal != NULL)
! elog(ERROR, "cursor \"%s\" already in use", name);
! }

/* Create the portal */
portal = CreatePortal(name);
***************
*** 1228,1233 ****
--- 1230,1236 ----
QueryDesc *querydesc;
EState *estate;
MemoryContext oldcontext;
+ CommandId savedId;
CommandDest olddest;

/* Check that the portal is valid */
***************
*** 1245,1250 ****
--- 1248,1254 ----

/* Switch to the portals memory context */
oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
+
querydesc = PortalGetQueryDesc(portal);
estate = PortalGetState(portal);

***************
*** 1253,1258 ****
--- 1257,1270 ----
olddest = querydesc->dest;
querydesc->dest = dest;

+ /*
+ * Restore the scanCommandId that was current when the cursor was
+ * opened. This ensures that we see the same tuples throughout the
+ * execution of the cursor.
+ */
+ savedId = GetScanCommandId();
+ SetScanCommandId(PortalGetCommandId(portal));
+
/* Run the executor like PerformPortalFetch and remember states */
if (forward)
{
***************
*** 1278,1283 ****
--- 1290,1300 ----
portal->atStart = true;
}
}
+
+ /*
+ * Restore outer command ID.
+ */
+ SetScanCommandId(savedId);

/* Restore the old command destination and switch back to callers */
/* memory context */
*** src/backend/utils/mmgr/portalmem.c.orig Thu Oct 25 01:49:51 2001
--- src/backend/utils/mmgr/portalmem.c Wed Feb 13 19:23:48 2002
***************
*** 168,173 ****
--- 168,174 ----

portal->queryDesc = queryDesc;
portal->attinfo = attinfo;
+ portal->commandId = GetScanCommandId();
portal->state = state;
portal->atStart = true; /* Allow fetch forward only */
portal->atEnd = false;
***************
*** 213,218 ****
--- 214,220 ----
/* initialize portal query */
portal->queryDesc = NULL;
portal->attinfo = NULL;
+ portal->commandId = 0;
portal->state = NULL;
portal->atStart = true; /* disallow fetches until query is set */
portal->atEnd = true;
*** src/include/utils/portal.h.orig Mon Nov 5 14:44:35 2001
--- src/include/utils/portal.h Wed Feb 13 19:23:42 2002
***************
*** 31,36 ****
--- 31,37 ----
MemoryContext heap; /* subsidiary memory */
QueryDesc *queryDesc; /* Info about query associated with portal */
TupleDesc attinfo;
+ CommandId commandId; /* Command counter value for query */
EState *state; /* Execution state of query */
bool atStart; /* T => fetch backwards is not allowed */
bool atEnd; /* T => fetch forwards is not allowed */
***************
*** 48,55 ****
*/
#define PortalGetQueryDesc(portal) ((portal)->queryDesc)
#define PortalGetTupleDesc(portal) ((portal)->attinfo)
! #define PortalGetState(portal) ((portal)->state)
! #define PortalGetHeapMemory(portal) ((portal)->heap)

/*
* estimate of the maximum number of open portals a user would have,
--- 49,57 ----
*/
#define PortalGetQueryDesc(portal) ((portal)->queryDesc)
#define PortalGetTupleDesc(portal) ((portal)->attinfo)
! #define PortalGetCommandId(portal) ((portal)->commandId)
! #define PortalGetState(portal) ((portal)->state)
! #define PortalGetHeapMemory(portal) ((portal)->heap)

/*
* estimate of the maximum number of open portals a user would have,

In response to

Browse pgsql-general by date

  From Date Subject
Next Message David Griffiths 2002-02-14 03:31:36 Re: Problem with null dates in 7.2
Previous Message Marc G. Fournier 2002-02-14 01:51:24 Re: Mail archives problems

Browse pgsql-hackers by date

  From Date Subject
Next Message Neil Conway 2002-02-14 05:59:40 Re: NAMEDATALEN Changes
Previous Message Tom Lane 2002-02-14 01:07:48 Re: geo_decls.h oopsie...

Browse pgsql-patches by date

  From Date Subject
Next Message Hiroshi Inoue 2002-02-14 16:53:17 Re: [GENERAL] Postgres 7.2 - Updating rows in cursor problem
Previous Message Rod Taylor 2002-02-14 01:13:26 Make equals sign optional in CREATE DATABASE WITH param = 'val'