Tweak portal (cursor) code so that it will not call the executor again
authorTom Lane
Tue, 27 Feb 2001 22:07:34 +0000 (22:07 +0000)
committerTom Lane
Tue, 27 Feb 2001 22:07:34 +0000 (22:07 +0000)
when user does another FETCH after reaching end of data, or another
FETCH backwards after reaching start.  This is needed because some plan
nodes are not very robust about being called again after they've already
returned NULL; for example, MergeJoin will crash in some states but not
others.  While the ideal approach would be for them all to handle this
correctly, it seems foolish to assume that no such bugs would creep in
again once cleaned up.  Therefore, the most robust answer is to prevent
the situation from arising at all.

src/backend/commands/command.c
src/backend/tcop/pquery.c
src/backend/utils/mmgr/portalmem.c
src/include/utils/portal.h

index 8808a03f1acb41361d687a487c40ac5d0b31ae08..8a3be15a052d063527c7923d7e980184b64b0ff4 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.121 2001/02/14 21:35:00 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/Attic/command.c,v 1.122 2001/02/27 22:07:34 tgl Exp $
  *
  * NOTES
  *   The PerformAddAttribute() code, like most of the relation
@@ -107,8 +107,8 @@ PerformPortalFetch(char *name,
                   CommandDest dest)
 {
    Portal      portal;
-   int         feature;
    QueryDesc  *queryDesc;
+   EState     *estate;
    MemoryContext oldcontext;
 
    /* ----------------
@@ -140,19 +140,14 @@ PerformPortalFetch(char *name,
    oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
 
    /* ----------------
-    *  setup "feature" to tell the executor which direction to go in.
-    * ----------------
-    */
-   if (forward)
-       feature = EXEC_FOR;
-   else
-       feature = EXEC_BACK;
-
-   /* ----------------
-    *  tell the destination to prepare to receive some tuples
+    *  tell the destination to prepare to receive some tuples.
+    *
+    *  If we've been asked for a MOVE, make a temporary QueryDesc
+    *  with the appropriate dummy destination.
     * ----------------
     */
    queryDesc = PortalGetQueryDesc(portal);
+   estate = PortalGetState(portal);
 
    if (dest == None)           /* MOVE */
    {
@@ -165,7 +160,7 @@ PerformPortalFetch(char *name,
 
    BeginCommand(name,
                 queryDesc->operation,
-                portal->attinfo, /* QueryDescGetTypeInfo(queryDesc) */
+                PortalGetTupleDesc(portal),
                 false,         /* portal fetches don't end up in
                                 * relations */
                 false,         /* this is a portal fetch, not a "retrieve
@@ -174,18 +169,45 @@ PerformPortalFetch(char *name,
                 dest);
 
    /* ----------------
-    *  execute the portal fetch operation
+    *  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 are
+    *  robust about being called again if they've already returned NULL
+    *  once.)  If it's OK to do the fetch, call the executor.  Then,
+    *  update the atStart/atEnd state depending on the number of tuples
+    *  that were retrieved.
     * ----------------
     */
-   ExecutorRun(queryDesc, PortalGetState(portal), feature, (long) count);
-
-   if (dest == None)           /* MOVE */
-       pfree(queryDesc);
+   if (forward)
+   {
+       if (! portal->atEnd)
+       {
+           ExecutorRun(queryDesc, estate, EXEC_FOR, (long) count);
+           if (estate->es_processed > 0)
+               portal->atStart = false; /* OK to back up now */
+           if (count <= 0 || (int) estate->es_processed < count)
+               portal->atEnd = true; /* we retrieved 'em all */
+       }
+   }
+   else
+   {
+       if (! portal->atStart)
+       {
+           ExecutorRun(queryDesc, estate, EXEC_BACK, (long) count);
+           if (estate->es_processed > 0)
+               portal->atEnd = false; /* OK to go forward now */
+           if (count <= 0 || (int) estate->es_processed < count)
+               portal->atStart = true; /* we retrieved 'em all */
+       }
+   }
 
    /* ----------------
-    * Switch back to old context.
+    *  Clean up and switch back to old context.
     * ----------------
     */
+   if (dest == None)           /* MOVE */
+       pfree(queryDesc);
+
    MemoryContextSwitchTo(oldcontext);
 
    /* ----------------
index 3481bc675ec3545a825fd32af38c40a687114ff4..b36b9f6510c766ee63e8a8b5a8ca946fc5c91477 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/tcop/pquery.c,v 1.41 2001/01/24 19:43:09 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/tcop/pquery.c,v 1.42 2001/02/27 22:07:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -248,7 +248,7 @@ ProcessQuery(Query *parsetree,
     * ----------------
     */
    if (isRetrieveIntoRelation)
-       queryDesc->dest = (int) None;
+       queryDesc->dest = None;
 
    /* ----------------
     *  create a default executor state.
index 08db60a492c26e6adb7aecd6fb59643703a4f404..63d3ed363cb94678a326b2fa08afcdf2e40ca450 100644 (file)
@@ -8,44 +8,26 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.39 2001/01/24 19:43:17 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/utils/mmgr/portalmem.c,v 1.40 2001/02/27 22:07:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 /*
  * NOTES
- *     Do not confuse "Portal" with "PortalEntry" (or "PortalBuffer").
- *     When a PQexec() routine is run, the resulting tuples
- *     find their way into a "PortalEntry".  The contents of the resulting
- *     "PortalEntry" can then be inspected by other PQxxx functions.
+ *     A "Portal" is a structure used to keep track of cursor queries.
  *
- *     A "Portal" is a structure used to keep track of queries of the
- *     form:
- *             retrieve portal FOO ( blah... ) where blah...
- *
- *     When the backend sees a "retrieve portal" query, it allocates
- *     a "PortalD" structure, plans the query and then stores the query
+ *     When the backend sees a "declare cursor" query, it allocates a
+ *     "PortalData" structure, plans the query and then stores the query
  *     in the portal without executing it.  Later, when the backend
  *     sees a
- *             fetch 1 into FOO
- *
+ *             fetch 1 from FOO
  *     the system looks up the portal named "FOO" in the portal table,
  *     gets the planned query and then calls the executor with a feature of
  *     '(EXEC_FOR 1).  The executor then runs the query and returns a single
  *     tuple.  The problem is that we have to hold onto the state of the
- *     portal query until we see a "close p".  This means we have to be
+ *     portal query until we see a "close".  This means we have to be
  *     careful about memory management.
  *
- *     Having said all that, here is what a PortalD currently looks like:
- *
- * struct PortalD {
- *     char*                           name;
- *     classObj(MemoryContext)         heap;
- *     List                            queryDesc;
- *     EState                          state;
- *     void                            (*cleanup) ARGS((Portal portal));
- * };
- *
  *     I hope this makes things clearer to whoever reads this -cim 2/22/91
  */
 
@@ -188,43 +170,13 @@ PortalSetQuery(Portal portal,
    AssertArg(IsA((Node *) state, EState));
 
    portal->queryDesc = queryDesc;
-   portal->state = state;
    portal->attinfo = attinfo;
+   portal->state = state;
+   portal->atStart = true;     /* Allow fetch forward only */
+   portal->atEnd = false;
    portal->cleanup = cleanup;
 }
 
-/*
- * PortalGetQueryDesc
- *     Returns query attached to portal.
- *
- * Exceptions:
- *     BadState if called when disabled.
- *     BadArg if portal is invalid.
- */
-QueryDesc  *
-PortalGetQueryDesc(Portal portal)
-{
-   AssertArg(PortalIsValid(portal));
-
-   return portal->queryDesc;
-}
-
-/*
- * PortalGetState
- *     Returns state attached to portal.
- *
- * Exceptions:
- *     BadState if called when disabled.
- *     BadArg if portal is invalid.
- */
-EState *
-PortalGetState(Portal portal)
-{
-   AssertArg(PortalIsValid(portal));
-
-   return portal->state;
-}
-
 /*
  * CreatePortal
  *     Returns a new portal given a name.
@@ -265,7 +217,8 @@ CreatePortal(char *name)
    portal->queryDesc = NULL;
    portal->attinfo = NULL;
    portal->state = NULL;
-
+   portal->atStart = true;     /* disallow fetches until query is set */
+   portal->atEnd = true;
    portal->cleanup = NULL;
 
    /* put portal in table */
@@ -315,13 +268,3 @@ AtEOXact_portals(void)
 {
    HashTableWalk(PortalHashTable, (HashtFunc) PortalDrop, 0);
 }
-
-/*
- * PortalGetHeapMemory
- *     Returns heap memory context for a given portal.
- */
-MemoryContext
-PortalGetHeapMemory(Portal portal)
-{
-   return portal->heap;
-}
index 9c09e8eaefee79ccb28f1857b986965796d5cbcc..27ef1bf433103ccc16c1bc4b1e3320461a04def7 100644 (file)
@@ -7,19 +7,14 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: portal.h,v 1.25 2001/01/24 19:43:28 momjian Exp $
+ * $Id: portal.h,v 1.26 2001/02/27 22:07:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 /*
  * Note:
  *     A portal is an abstraction which represents the execution state of
- * a running query (or a fixed sequence of queries).
- *
- * Note:
- *     now that PQ calls can be made from within a backend, a portal
- *     may also be used to keep track of the tuples resulting
- *     from the execution of a query.  In this case, entryIndex
+ * a running query (specifically, a CURSOR).
  */
 #ifndef PORTAL_H
 #define PORTAL_H
 #include "nodes/memnodes.h"
 
 
-typedef struct PortalD *Portal;
+typedef struct PortalData *Portal;
 
-typedef struct PortalD
+typedef struct PortalData
 {
    char       *name;           /* Portal's name */
    MemoryContext heap;         /* subsidiary memory */
    QueryDesc  *queryDesc;      /* Info about query associated with portal */
    TupleDesc   attinfo;
-   EState     *state;
+   EState     *state;          /* Execution state of query */
+   bool        atStart;        /* T => fetch backwards is not allowed */
+   bool        atEnd;          /* T => fetch forwards is not allowed */
    void        (*cleanup) (Portal); /* Cleanup routine (optional) */
-} PortalD;
+} PortalData;
 
 /*
  * PortalIsValid
@@ -46,6 +43,21 @@ typedef struct PortalD
  */
 #define PortalIsValid(p) PointerIsValid(p)
 
+/*
+ * Access macros for Portal ... use these in preference to field access.
+ */
+#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,
+ * used in initially sizing the PortalHashTable in EnablePortalManager()
+ */
+#define PORTALS_PER_USER      10
+
+
 extern void EnablePortalManager(void);
 extern void AtEOXact_portals(void);
 extern Portal CreatePortal(char *name);
@@ -54,14 +66,6 @@ extern Portal GetPortalByName(char *name);
 extern void PortalSetQuery(Portal portal, QueryDesc *queryDesc,
               TupleDesc attinfo, EState *state,
               void (*cleanup) (Portal portal));
-extern QueryDesc *PortalGetQueryDesc(Portal portal);
-extern EState *PortalGetState(Portal portal);
-extern MemoryContext PortalGetHeapMemory(Portal portal);
-
-/* estimate of the maximum number of open portals a user would have,
- * used in initially sizing the PortalHashTable in EnablePortalManager()
- */
-#define PORTALS_PER_USER      10
 
 
 #endif  /* PORTAL_H */