In a PL/pgSQL "FOR cursor" statement, the statements executed in the loop

might close the cursor,  rendering the Portal pointer to it invalid.
Closing the cursor in the middle of the loop is not a very sensible thing
to do, but we must handle it gracefully and throw an error instead of
crashing.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 8f5e97b240f02a0cfc9d19a2426daafdcc6dd7d4..2931b2c4f79081db0b00de81d3272e0cfb1a3ddb 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.244.2.4 2010/04/14 23:52:16 tgl Exp $
+ *   $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.244.2.5 2010/06/21 09:49:58 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1869,6 +1869,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
 {
    PLpgSQL_var *curvar;
    char       *curname = NULL;
+   const char   *portalname;
    PLpgSQL_expr *query;
    Portal      portal;
    int         rc;
@@ -1951,6 +1952,7 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
    if (portal == NULL)
        elog(ERROR, "could not open cursor: %s",
             SPI_result_code_string(SPI_result));
+   portalname = portal->name;
 
    /*
     * If cursor variable was NULL, store the generated portal name in it
@@ -1965,11 +1967,20 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
    rc = exec_for_query(estate, (PLpgSQL_stmt_forq *) stmt, portal, false);
 
    /* ----------
-    * Close portal, and restore cursor variable if it was initially NULL.
+    * Close portal. The statements executed in the loop might've closed the
+    * cursor already, rendering our portal pointer invalid, so we mustn't
+    * trust the pointer.
     * ----------
     */
+   portal = SPI_cursor_find(portalname);
+   if (portal == NULL)
+       ereport(ERROR,
+               (errcode(ERRCODE_UNDEFINED_CURSOR),
+                errmsg("cursor \"%s\" closed unexpectedly",
+                       portalname)));
    SPI_cursor_close(portal);
 
+   /* Restore cursor variable if it was initially NULL. */
    if (curname == NULL)
    {
        free_var(curvar);
@@ -4189,6 +4200,13 @@ exec_run_select(PLpgSQL_execstate *estate,
  * exec_for_query --- execute body of FOR loop for each row from a portal
  *
  * Used by exec_stmt_fors, exec_stmt_forc and exec_stmt_dynfors
+ *
+ * If the portal is for a cursor that's visible to user code, the statements
+ * we execute might move or close the cursor. You must pass prefetch_ok=false
+ * in that case to disable optimizations that rely on the portal staying
+ * unchanged over execution of the user statements.
+ * NB: With prefetch_ok=false, the portal pointer might point to garbage
+ * after the call. Caller beware!
  */
 static int
 exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
@@ -4200,6 +4218,10 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
    bool        found = false;
    int         rc = PLPGSQL_RC_OK;
    int         n;
+   const char *portalname;
+
+   /* Remember portal name so that we can re-find it */
+   portalname = portal->name;
 
    /*
     * Determine if we assign to a record or a row
@@ -4308,8 +4330,22 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
 
        /*
         * Fetch more tuples.  If prefetching is allowed, grab 50 at a time.
+        * Otherwise the statements executed in the loop might've moved or
+        * even closed the cursor, so check that the cursor is still open,
+        * and fetch only one row at a time.
         */
-       SPI_cursor_fetch(portal, true, prefetch_ok ? 50 : 1);
+       if (prefetch_ok)
+           SPI_cursor_fetch(portal, true, 50);
+       else
+       {
+           portal = SPI_cursor_find(portalname);
+           if (portal == NULL)
+               ereport(ERROR,
+                       (errcode(ERRCODE_UNDEFINED_CURSOR),
+                        errmsg("cursor \"%s\" closed unexpectedly",
+                               portalname)));
+           SPI_cursor_fetch(portal, true, 1);
+       }
        tuptab = SPI_tuptable;
        n = SPI_processed;
    }