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.

5 files changed, 113 insertions, 74 deletions

diff --git a/doc/src/sgml/ref/fetch.sgml b/doc/src/sgml/ref/fetch.sgml
index b08ad4a191e..b67b7ef3c7a 100644
--- 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>
@@ -107,6 +107,17 @@ FETCH [ FORWARD | BACKWARD | RELATIVE ] [ <replaceable class="PARAMETER">#</repl
 
 	 <varlistentry>
 	  <term>
+	   LAST
+	  </term>
+	  <listitem>
+	   <para>
+	    Same as <literal>ALL</>, but conforms to SQL92 syntax.
+	   </para>
+	  </listitem>
+	 </varlistentry>
+
+	 <varlistentry>
+	  <term>
 	   NEXT
 	  </term>
 	  <listitem>
@@ -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 5d4f1c8309a..5b6f2671af8 100644
--- 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 2f3295cab87..55007a7d627 100644
--- 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 3a670d8f899..5881fe6c582 100644
--- 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 921099b7926..1809d515461 100644
--- 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