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 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/ref/fetch.sgml,v 1.22 2002/12/30 15:31:47 momjian Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/ref/fetch.sgml,v 1.23 2003/01/08 00:22:26 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -22,7 +22,7 @@ PostgreSQL documentation
   </refsynopsisdivinfo>
   <synopsis>
 FETCH [ <replaceable class="PARAMETER">direction</replaceable> ] [ <replaceable class="PARAMETER">count</replaceable> ] { IN | FROM } <replaceable class="PARAMETER">cursor</replaceable>
-FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</replaceable> | ALL | NEXT | PRIOR ]
+FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</replaceable> | ALL | LAST | NEXT | PRIOR ]
     { IN | FROM } <replaceable class="PARAMETER">cursor</replaceable>
   </synopsis>
 
@@ -40,7 +40,7 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
       <term><replaceable class="PARAMETER">direction</replaceable></term>
       <listitem>
        <para>
-       <replaceable class="PARAMETER">selector</replaceable>
+       <replaceable class="PARAMETER">direction</replaceable>
        defines the fetch direction. It can be one of
        the following:
 
@@ -50,7 +50,7 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
          <listitem>
           <para>
            fetch next row(s). This is the default
-           if <replaceable class="PARAMETER">selector</replaceable> is omitted.
+           if <replaceable class="PARAMETER">direction</replaceable> is omitted.
           </para>
          </listitem>
         </varlistentry>
@@ -87,9 +87,9 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
          <term><replaceable class="PARAMETER">#</replaceable></term>
          <listitem>
           <para>
-           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.
           </para>
          </listitem>
         </varlistentry>
@@ -105,6 +105,17 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
          </listitem>
         </varlistentry>
 
+        <varlistentry>
+         <term>
+          LAST
+         </term>
+         <listitem>
+          <para>
+           Same as <literal>ALL</>, but conforms to SQL92 syntax.
+          </para>
+         </listitem>
+        </varlistentry>
+
         <varlistentry>
          <term>
           NEXT
@@ -151,7 +162,8 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
     Outputs
    </title>
    <para>
-    <command>FETCH</command> returns the results of the query defined by the specified cursor.
+    <command>FETCH</command> returns rows from the result of the query defined
+    by the specified cursor.
     The following messages will be returned if the query fails:
 
     <variablelist>
@@ -200,10 +212,33 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    If the number of rows remaining in the cursor is less
    than <replaceable class="PARAMETER">#</replaceable>,
    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.
+  </para>
+
+  <para>
+   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
+   <command>FETCH</command> always steps one row in the specified direction
+   (if possible) before beginning to return rows.  If the
+   <command>FETCH</command> 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
+   <command>FETCH ALL</>.
+  </para>
+
+   <tip>
+    <para>
+     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.
+    </para>
+   </tip>
 
    <tip>
     <para>
@@ -213,7 +248,6 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
      <command>FORWARD -1</command> is the same as <command>BACKWARD 1</command>.
     </para>
    </tip>
-  </para>
 
   <refsect2 id="R2-SQL-FETCH-3">
    <refsect2info>
@@ -224,11 +258,9 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    </title>
 
    <para>
-    Note that the FORWARD and BACKWARD keywords are
+    Note that the FORWARD, BACKWARD, and ALL keywords are
     <productname>PostgreSQL</productname> extensions.
-    The <acronym>SQL92</acronym> 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.
    </para>
 
    <para>
@@ -246,11 +278,11 @@ WARNING:  FETCH/ABSOLUTE not supported, using RELATIVE
    </para>
 
    <para>
+    <xref linkend="sql-declare" endterm="sql-declare-title">
+    is used to define a cursor.
     Use
     <xref linkend="sql-move" endterm="sql-move-title">
-    to change cursor position.
-    <xref linkend="sql-declare" endterm="sql-declare-title">
-    will define a cursor.
+    to change cursor position without retrieving data.
     Refer to
     <xref linkend="sql-begin" endterm="sql-begin-title">,
     <xref linkend="sql-commit" endterm="sql-commit-title">,

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 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/ref/move.sgml,v 1.15 2002/12/30 15:31:47 momjian Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/ref/move.sgml,v 1.16 2003/01/08 00:22:26 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -13,7 +13,7 @@ PostgreSQL documentation
    MOVE
   </refname>
   <refpurpose>
-   position a cursor on a specified row of a table
+   reposition a cursor
   </refpurpose>
  </refnamediv>                
  <refsynopsisdiv>
@@ -21,9 +21,7 @@ PostgreSQL documentation
    <date>1999-07-20</date>
   </refsynopsisdivinfo>
   <synopsis>
-MOVE [ <replaceable class="PARAMETER">direction</replaceable> ] 
-    {<replaceable class="PARAMETER">count</replaceable> | LAST }
-    { IN | FROM } <replaceable class="PARAMETER">cursor</replaceable>
+MOVE [ <replaceable class="PARAMETER">direction</replaceable> ] [ <replaceable class="PARAMETER">count</replaceable> ] { IN | FROM } <replaceable class="PARAMETER">cursor</replaceable>
   </synopsis>
  </refsynopsisdiv>
 
@@ -35,12 +33,10 @@ MOVE [ <replaceable class="PARAMETER">direction</replaceable> ]
    Description
   </title>
   <para>
-   <command>MOVE</command> allows a user to move the cursor position a 
-   specified number of rows.
-   <command>MOVE</command> works like the <command>FETCH</command> command,
-   but only positions the cursor and does not return rows.
-   <replaceable class="PARAMETER">LAST</replaceable> moves to the end
-   of the cursor.
+   <command>MOVE</command> 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.
+   <command>MOVE</command> works exactly like the <command>FETCH</command>
+   command, except it only repositions the cursor and does not return rows.
   </para>
   <para>
    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 @@
 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.175 2003/01/06 18:53:23 petere Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/release.sgml,v 1.176 2003/01/08 00:22:26 tgl Exp $
 -->
 
 <appendix id="release">
@@ -24,6 +24,7 @@ CDATA means the content is "SGML-free", so you can write without
 worries about funny characters.
 -->
 <literallayout><![CDATA[
+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