Code review for FETCH/MOVE 0 changes. Improve documentation, do the

right thing with the destination when FETCH 0 can't return a row,
don't try to stuff LONG_MAX into an int value.

diff --git a/doc/src/sgml/ref/fetch.sgml b/doc/src/sgml/ref/fetch.sgml
index b08ad4a191e8141314d967a95163688448e87f14..b67b7ef3c7a9529fa6a72217b87909373607e3eb 100644 (file)
--- a/doc/src/sgml/ref/fetch.sgml
+++ b/doc/src/sgml/ref/fetch.sgml
@@ -1,5 +1,5 @@
 
 
@@ -22,7 +22,7 @@ PostgreSQL documentation
   
   
 FETCH [ direction ] [ count ] { IN | FROM } cursor
-FETCH [ FORWARD | BACKWARD | RELATIVE ] [ # | ALL | NEXT | PRIOR ]
+FETCH [ FORWARD | BACKWARD | RELATIVE ] [ # | ALL | LAST | NEXT | PRIOR ]
     { IN | FROM } cursor
   
 
@@ -40,7 +40,7 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ #
       direction
       
        
-   selector
+   direction
    defines the fetch direction. It can be one of
    the following:
 
@@ -50,7 +50,7 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ #
      
       
        fetch next row(s). This is the default
-       if selector is omitted.
+       if direction is omitted.
       
      
     
@@ -87,9 +87,9 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ #
      #
      
       
-       A signed integer that specifies how many rows to fetch.
+       A signed integer constant that specifies how many rows to fetch.
        Note that a negative integer is equivalent to changing the sense of
-       FORWARD and BACKWARD. Zero re-fetches the current row.
+       FORWARD and BACKWARD. Zero re-fetches the current row, if any.
       
      
     
@@ -105,6 +105,17 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ #
      
     
 
+    
+     
+      LAST
+     
+     
+      
+       Same as ALL, but conforms to SQL92 syntax.
+      
+     
+    
+
     
      
       NEXT
@@ -151,7 +162,8 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ #
     Outputs
    
    
-    FETCH returns the results of the query defined by the specified cursor.
+    FETCH returns rows from the result of the query defined
+    by the specified cursor.
     The following messages will be returned if the query fails:
 
     
@@ -200,10 +212,33 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    If the number of rows remaining in the cursor is less
    than #,
    then only those available are fetched.
-   Substituting the keyword ALL in place of a number will
+   Substituting the keyword ALL or LAST in place of a number will
    cause all remaining rows in the cursor to be retrieved.
-   Instances may be fetched in both FORWARD and BACKWARD
+   Rows may be fetched in both FORWARD and BACKWARD
    directions. The default direction is FORWARD.
+  
+
+  
+   The cursor position can be before the first row of the query result, or on
+   any particular row of the result, or after the last row of the result.
+   When created, a cursor is positioned before the first row.  After fetching
+   some rows, the cursor is positioned on the last row retrieved.  A new
+   FETCH always steps one row in the specified direction
+   (if possible) before beginning to return rows.  If the
+   FETCH requests more rows than available, the cursor is
+   left positioned after the last row of the query result (or before the first
+   row, in the case of a backward fetch).  This will always be the case after
+   FETCH ALL.
+  
+
+   
+    
+     A zero row count requests fetching the current row without moving the
+     cursor --- that is, re-fetching the most recently fetched row.
+     This will succeed unless the cursor is positioned before the
+     first row or after the last row; in which case, no row is returned.
+    
+   
 
    
     
@@ -213,7 +248,6 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
      FORWARD -1 is the same as BACKWARD 1.
     
    
-  
 
   
    
@@ -224,11 +258,9 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    
 
    
-    Note that the FORWARD and BACKWARD keywords are
+    Note that the FORWARD, BACKWARD, and ALL keywords are
     PostgreSQL extensions.
-    The SQL92 syntax is also supported, specified
-    in the second form of the command. See below for details
-    on compatibility issues.
+    See below for details on compatibility issues.
    
 
    
@@ -246,11 +278,11 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    
 
    
+    
+    is used to define a cursor.
     Use
     
-    to change cursor position.
-    
-    will define a cursor.
+    to change cursor position without retrieving data.
     Refer to
     ,
     ,

diff --git a/doc/src/sgml/ref/move.sgml b/doc/src/sgml/ref/move.sgml
index 5d4f1c8309ae872866997f9cd619efcd4a6ede52..5b6f2671af8ee8a6f24c2e017e5ec468238f8e13 100644 (file)
--- a/doc/src/sgml/ref/move.sgml
+++ b/doc/src/sgml/ref/move.sgml
@@ -1,5 +1,5 @@
 
 
@@ -13,7 +13,7 @@ PostgreSQL documentation
    MOVE
   
   
-   position a cursor on a specified row of a table
+   reposition a cursor
   
                  
  
@@ -21,9 +21,7 @@ PostgreSQL documentation
    1999-07-20
   
   
-MOVE [ direction ] 
-    {count | LAST }
-    { IN | FROM } cursor
+MOVE [ direction ] [ count ] { IN | FROM } cursor
   
  
 
@@ -35,12 +33,10 @@ MOVE [ direction ]
    Description
   
   
-   MOVE allows a user to move the cursor position a 
-   specified number of rows.
-   MOVE works like the FETCH command,
-   but only positions the cursor and does not return rows.
-   LAST moves to the end
-   of the cursor.
+   MOVE allows the user to move the cursor position a 
+   specified number of rows, or all the way to the end or start of the query.
+   MOVE works exactly like the FETCH
+   command, except it only repositions the cursor and does not return rows.
   
   
    Refer to 

diff --git a/doc/src/sgml/release.sgml b/doc/src/sgml/release.sgml
index 2f3295cab87b790566f7b486464ce501fdfe95b4..55007a7d627e8ac8c0ae0a18f440b4d24eb31de6 100644 (file)
--- a/doc/src/sgml/release.sgml
+++ b/doc/src/sgml/release.sgml
@@ -1,5 +1,5 @@
 
 
 
@@ -24,6 +24,7 @@ CDATA means the content is "SGML-free", so you can write without
 worries about funny characters.
 -->
 
+FETCH 0 now re-fetches cursor's current row, per SQL spec
 Revised executor state representation; plan trees are read-only to executor now
 Information schema
 Domains now support CHECK constraints


diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c

index 3a670d8f899460b581afef1d824ace37f8cdb226..5881fe6c582288f74b894de41d2389b433ab1f11 100644 (file)

--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v 1.7 2002/12/30 15:31:47 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v 1.8 2003/01/08 00:22:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,7 +55,7 @@ PortalCleanup(Portal portal)
  *
  * name: name of portal
  * forward: forward or backward fetch?
- * count: # of tuples to fetch
+ * count: # of tuples to fetch (INT_MAX means "all"; 0 means "refetch")
  * dest: where to send results
  * completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE
  *     in which to store a command completion status string.
@@ -100,18 +100,15 @@ PerformPortalFetch(char *name,
        return;
    }
 
-   /* If zero count, handle specially */
+   /*
+    * Zero count means to re-fetch the current row, if any (per SQL92)
+    */
    if (count == 0)
    {
-       bool on_row = false;
+       bool    on_row;
 
        /* Are we sitting on a row? */
-       oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
-       queryDesc = PortalGetQueryDesc(portal);
-       estate = queryDesc->estate;
-       if (portal->atStart == false && portal->atEnd == false)
-           on_row = true;
-       MemoryContextSwitchTo(oldcontext);
+       on_row = (portal->atStart == false && portal->atEnd == false);
 
        if (dest == None)
        {
@@ -122,26 +119,25 @@ PerformPortalFetch(char *name,
        }
        else
        {
-           /* If we are not on a row, FETCH 0 returns nothing */
-           if (!on_row)
-               return;
-
-           /* Since we are sitting on a row, return the row */
-           /* Back up so we can reread the row */
-           PerformPortalFetch(name, false /* backward */, 1,
-                              None, /* throw away output */
-                              NULL /* do not modify the command tag */);
-
-           /* Set up to fetch one row */
-           count = 1;
-           forward = true;
+           /*
+            * If we are sitting on a row, back up one so we can re-fetch it.
+            * If we are not sitting on a row, we still have to start up and
+            * shut down the executor so that the destination is initialized
+            * and shut down correctly; so keep going.  Further down in the
+            * routine, count == 0 means we will retrieve no row.
+            */
+           if (on_row)
+           {
+               PerformPortalFetch(name, false /* backward */, 1L,
+                                  None, /* throw away output */
+                                  NULL /* do not modify the command tag */);
+               /* Set up to fetch one row forward */
+               count = 1;
+               forward = true;
+           }
        }
    }
 
-   /* Internally, zero count processes all portal rows */
-   if (count == LONG_MAX)
-       count = 0;
-
    /*
     * switch into the portal context
     */
@@ -185,31 +181,45 @@ PerformPortalFetch(char *name,
     */
    if (forward)
    {
-       if (portal->atEnd)
+       if (portal->atEnd || count == 0)
            direction = NoMovementScanDirection;
        else
            direction = ForwardScanDirection;
 
-       ExecutorRun(queryDesc, direction, (long) count);
+       /* In the executor, zero count processes all portal rows */
+       if (count == INT_MAX)
+           count = 0;
 
-       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 */
+       ExecutorRun(queryDesc, direction, count);
+
+       if (direction != NoMovementScanDirection)
+       {
+           if (estate->es_processed > 0)
+               portal->atStart = false;    /* OK to back up now */
+           if (count <= 0 || (long) estate->es_processed < count)
+               portal->atEnd = true;       /* we retrieved 'em all */
+       }
    }
    else
    {
-       if (portal->atStart)
+       if (portal->atStart || count == 0)
            direction = NoMovementScanDirection;
        else
            direction = BackwardScanDirection;
 
-       ExecutorRun(queryDesc, direction, (long) count);
+       /* In the executor, zero count processes all portal rows */
+       if (count == INT_MAX)
+           count = 0;
+
+       ExecutorRun(queryDesc, direction, 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 */
+       if (direction != NoMovementScanDirection)
+       {
+           if (estate->es_processed > 0)
+               portal->atEnd = false;      /* OK to go forward now */
+           if (count <= 0 || (long) estate->es_processed < count)
+               portal->atStart = true;     /* we retrieved 'em all */
+       }
    }
 
    /* Return command status if wanted */


diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y

index 921099b79267768871a172d4aef9a8b3723ef054..1809d515461d8e0081481c4ee576c1221a186e6c 100644 (file)

--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.390 2003/01/06 00:31:44 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.391 2003/01/08 00:22:27 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -2594,7 +2594,7 @@ FetchStmt:    FETCH direction fetch_how_many from_in name
                    if ($3 < 0)
                    {
                        $3 = -$3;
-                       $2 = (($2 == FORWARD)? BACKWARD: FORWARD);
+                       $2 = (($2 == FORWARD) ? BACKWARD : FORWARD);
                    }
                    n->direction = $2;
                    n->howMany = $3;
@@ -2652,7 +2652,7 @@ FetchStmt:    FETCH direction fetch_how_many from_in name
                    if ($3 < 0)
                    {
                        $3 = -$3;
-                       $2 = (($2 == FORWARD) ? BACKWARD: FORWARD);
+                       $2 = (($2 == FORWARD) ? BACKWARD : FORWARD);
                    }
                    n->direction = $2;
                    n->howMany = $3;
@@ -2720,8 +2720,8 @@ direction:    FORWARD                                 { $$ = FORWARD; }
 fetch_how_many:
            Iconst                                  { $$ = $1; }
            | '-' Iconst                            { $$ = - $2; }
-           | ALL                                   { $$ = LONG_MAX; }
-           | LAST                                  { $$ = LONG_MAX; }
+           | ALL                                   { $$ = INT_MAX; }
+           | LAST                                  { $$ = INT_MAX; }
            | NEXT                                  { $$ = 1; }
            | PRIOR                                 { $$ = -1; }
        ;
@@ -7115,8 +7115,8 @@ unreserved_keyword:
            | INVOKER
            | ISOLATION
            | KEY
-           | LANGUAGE
            | LANCOMPILER
+           | LANGUAGE
            | LAST
            | LEVEL
            | LISTEN

diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 3a670d8f899460b581afef1d824ace37f8cdb226..5881fe6c582288f74b894de41d2389b433ab1f11 100644 (file)
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v 1.7 2002/12/30 15:31:47 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v 1.8 2003/01/08 00:22:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,7 +55,7 @@ PortalCleanup(Portal portal)
  *
  * name: name of portal
  * forward: forward or backward fetch?
- * count: # of tuples to fetch
+ * count: # of tuples to fetch (INT_MAX means "all"; 0 means "refetch")
  * dest: where to send results
  * completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE
  *     in which to store a command completion status string.
@@ -100,18 +100,15 @@ PerformPortalFetch(char *name,
        return;
    }
 
-   /* If zero count, handle specially */
+   /*
+    * Zero count means to re-fetch the current row, if any (per SQL92)
+    */
    if (count == 0)
    {
-       bool on_row = false;
+       bool    on_row;
 
        /* Are we sitting on a row? */
-       oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
-       queryDesc = PortalGetQueryDesc(portal);
-       estate = queryDesc->estate;
-       if (portal->atStart == false && portal->atEnd == false)
-           on_row = true;
-       MemoryContextSwitchTo(oldcontext);
+       on_row = (portal->atStart == false && portal->atEnd == false);
 
        if (dest == None)
        {
@@ -122,26 +119,25 @@ PerformPortalFetch(char *name,
        }
        else
        {
-           /* If we are not on a row, FETCH 0 returns nothing */
-           if (!on_row)
-               return;
-
-           /* Since we are sitting on a row, return the row */
-           /* Back up so we can reread the row */
-           PerformPortalFetch(name, false /* backward */, 1,
-                              None, /* throw away output */
-                              NULL /* do not modify the command tag */);
-
-           /* Set up to fetch one row */
-           count = 1;
-           forward = true;
+           /*
+            * If we are sitting on a row, back up one so we can re-fetch it.
+            * If we are not sitting on a row, we still have to start up and
+            * shut down the executor so that the destination is initialized
+            * and shut down correctly; so keep going.  Further down in the
+            * routine, count == 0 means we will retrieve no row.
+            */
+           if (on_row)
+           {
+               PerformPortalFetch(name, false /* backward */, 1L,
+                                  None, /* throw away output */
+                                  NULL /* do not modify the command tag */);
+               /* Set up to fetch one row forward */
+               count = 1;
+               forward = true;
+           }
        }
    }
 
-   /* Internally, zero count processes all portal rows */
-   if (count == LONG_MAX)
-       count = 0;
-
    /*
     * switch into the portal context
     */
@@ -185,31 +181,45 @@ PerformPortalFetch(char *name,
     */
    if (forward)
    {
-       if (portal->atEnd)
+       if (portal->atEnd || count == 0)
            direction = NoMovementScanDirection;
        else
            direction = ForwardScanDirection;
 
-       ExecutorRun(queryDesc, direction, (long) count);
+       /* In the executor, zero count processes all portal rows */
+       if (count == INT_MAX)
+           count = 0;
 
-       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 */
+       ExecutorRun(queryDesc, direction, count);
+
+       if (direction != NoMovementScanDirection)
+       {
+           if (estate->es_processed > 0)
+               portal->atStart = false;    /* OK to back up now */
+           if (count <= 0 || (long) estate->es_processed < count)
+               portal->atEnd = true;       /* we retrieved 'em all */
+       }
    }
    else
    {
-       if (portal->atStart)
+       if (portal->atStart || count == 0)
            direction = NoMovementScanDirection;
        else
            direction = BackwardScanDirection;
 
-       ExecutorRun(queryDesc, direction, (long) count);
+       /* In the executor, zero count processes all portal rows */
+       if (count == INT_MAX)
+           count = 0;
+
+       ExecutorRun(queryDesc, direction, 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 */
+       if (direction != NoMovementScanDirection)
+       {
+           if (estate->es_processed > 0)
+               portal->atEnd = false;      /* OK to go forward now */
+           if (count <= 0 || (long) estate->es_processed < count)
+               portal->atStart = true;     /* we retrieved 'em all */
+       }
    }
 
    /* Return command status if wanted */

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 921099b79267768871a172d4aef9a8b3723ef054..1809d515461d8e0081481c4ee576c1221a186e6c 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.390 2003/01/06 00:31:44 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.391 2003/01/08 00:22:27 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -2594,7 +2594,7 @@ FetchStmt:    FETCH direction fetch_how_many from_in name
                    if ($3 < 0)
                    {
                        $3 = -$3;
-                       $2 = (($2 == FORWARD)? BACKWARD: FORWARD);
+                       $2 = (($2 == FORWARD) ? BACKWARD : FORWARD);
                    }
                    n->direction = $2;
                    n->howMany = $3;
@@ -2652,7 +2652,7 @@ FetchStmt:    FETCH direction fetch_how_many from_in name
                    if ($3 < 0)
                    {
                        $3 = -$3;
-                       $2 = (($2 == FORWARD) ? BACKWARD: FORWARD);
+                       $2 = (($2 == FORWARD) ? BACKWARD : FORWARD);
                    }
                    n->direction = $2;
                    n->howMany = $3;
@@ -2720,8 +2720,8 @@ direction:    FORWARD                                 { $$ = FORWARD; }
 fetch_how_many:
            Iconst                                  { $$ = $1; }
            | '-' Iconst                            { $$ = - $2; }
-           | ALL                                   { $$ = LONG_MAX; }
-           | LAST                                  { $$ = LONG_MAX; }
+           | ALL                                   { $$ = INT_MAX; }
+           | LAST                                  { $$ = INT_MAX; }
            | NEXT                                  { $$ = 1; }
            | PRIOR                                 { $$ = -1; }
        ;
@@ -7115,8 +7115,8 @@ unreserved_keyword:
            | INVOKER
            | ISOLATION
            | KEY
-           | LANGUAGE
            | LANCOMPILER
+           | LANGUAGE
            | LAST
            | LEVEL
            | LISTEN