Redesign internal logic of nodeLimit so that it does not need to fetch

one more row from the subplan than the COUNT would appear to require.
This costs a little more logic but a number of people have complained
about the old implementation.

src/backend/executor/nodeLimit.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeLimit.c,v 1.10 2002/06/20 20:29:28 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeLimit.c,v 1.11 2002/11/22 22:10:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,7 +42,6 @@ ExecLimit(Limit *node)
 	TupleTableSlot *resultTupleSlot;
 	TupleTableSlot *slot;
 	Plan	   *outerPlan;
-	long		netlimit;
 
 	/*
 	 * get information from the node
@@ -53,93 +52,160 @@ ExecLimit(Limit *node)
 	resultTupleSlot = limitstate->cstate.cs_ResultTupleSlot;
 
 	/*
-	 * If first call for this scan, compute limit/offset. (We can't do
-	 * this any earlier, because parameters from upper nodes may not be
-	 * set until now.)
+	 * The main logic is a simple state machine.
 	 */
-	if (!limitstate->parmsSet)
-		recompute_limits(node);
-	netlimit = limitstate->offset + limitstate->count;
-
-	/*
-	 * now loop, returning only desired tuples.
-	 */
-	for (;;)
+	switch (limitstate->lstate)
 	{
-		/*
-		 * If we have reached the subplan EOF or the limit, just quit.
-		 *
-		 * NOTE: when scanning forwards, we must fetch one tuple beyond the
-		 * COUNT limit before we can return NULL, else the subplan won't
-		 * be properly positioned to start going backwards.  Hence test
-		 * here is for position > netlimit not position >= netlimit.
-		 *
-		 * Similarly, when scanning backwards, we must re-fetch the last
-		 * tuple in the offset region before we can return NULL. Otherwise
-		 * we won't be correctly aligned to start going forward again. So,
-		 * although you might think we can quit when position equals
-		 * offset + 1, we have to fetch a subplan tuple first, and then
-		 * exit when position = offset.
-		 */
-		if (ScanDirectionIsForward(direction))
-		{
-			if (limitstate->atEnd)
-				return NULL;
-			if (!limitstate->noCount && limitstate->position > netlimit)
+		case LIMIT_INITIAL:
+			/*
+			 * If backwards scan, just return NULL without changing state.
+			 */
+			if (!ScanDirectionIsForward(direction))
 				return NULL;
-		}
-		else
-		{
-			if (limitstate->position <= limitstate->offset)
+			/*
+			 * First call for this scan, so compute limit/offset. (We can't do
+			 * this any earlier, because parameters from upper nodes may not
+			 * be set until now.)  This also sets position = 0.
+			 */
+			recompute_limits(node);
+			/*
+			 * Check for empty window; if so, treat like empty subplan.
+			 */
+			if (limitstate->count <= 0 && !limitstate->noCount)
+			{
+				limitstate->lstate = LIMIT_EMPTY;
 				return NULL;
-		}
-
-		/*
-		 * fetch a tuple from the outer subplan
-		 */
-		slot = ExecProcNode(outerPlan, (Plan *) node);
-		if (TupIsNull(slot))
-		{
+			}
 			/*
-			 * We are at start or end of the subplan.  Update local state
-			 * appropriately, but always return NULL.
+			 * Fetch rows from subplan until we reach position > offset.
 			 */
+			for (;;)
+			{
+				slot = ExecProcNode(outerPlan, (Plan *) node);
+				if (TupIsNull(slot))
+				{
+					/*
+					 * The subplan returns too few tuples for us to produce
+					 * any output at all.
+					 */
+					limitstate->lstate = LIMIT_EMPTY;
+					return NULL;
+				}
+				limitstate->subSlot = slot;
+				if (++limitstate->position > limitstate->offset)
+					break;
+			}
+			/*
+			 * Okay, we have the first tuple of the window.
+			 */
+			limitstate->lstate = LIMIT_INWINDOW;
+			break;
+
+		case LIMIT_EMPTY:
+			/*
+			 * The subplan is known to return no tuples (or not more than
+			 * OFFSET tuples, in general).  So we return no tuples.
+			 */
+			return NULL;
+
+		case LIMIT_INWINDOW:
 			if (ScanDirectionIsForward(direction))
 			{
-				Assert(!limitstate->atEnd);
-				/* must bump position to stay in sync for backwards fetch */
+				/*
+				 * Forwards scan, so check for stepping off end of window.
+				 * If we are at the end of the window, return NULL without
+				 * advancing the subplan or the position variable; but
+				 * change the state machine state to record having done so.
+				 */
+				if (!limitstate->noCount &&
+					limitstate->position >= limitstate->offset + limitstate->count)
+				{
+					limitstate->lstate = LIMIT_WINDOWEND;
+					return NULL;
+				}
+				/*
+				 * Get next tuple from subplan, if any.
+				 */
+				slot = ExecProcNode(outerPlan, (Plan *) node);
+				if (TupIsNull(slot))
+				{
+					limitstate->lstate = LIMIT_SUBPLANEOF;
+					return NULL;
+				}
+				limitstate->subSlot = slot;
 				limitstate->position++;
-				limitstate->atEnd = true;
 			}
 			else
 			{
-				limitstate->position = 0;
-				limitstate->atEnd = false;
+				/*
+				 * Backwards scan, so check for stepping off start of window.
+				 * As above, change only state-machine status if so.
+				 */
+				if (limitstate->position <= limitstate->offset + 1)
+				{
+					limitstate->lstate = LIMIT_WINDOWSTART;
+					return NULL;
+				}
+				/*
+				 * Get previous tuple from subplan; there should be one!
+				 */
+				slot = ExecProcNode(outerPlan, (Plan *) node);
+				if (TupIsNull(slot))
+					elog(ERROR, "ExecLimit: subplan failed to run backwards");
+				limitstate->subSlot = slot;
+				limitstate->position--;
 			}
-			return NULL;
-		}
-
-		/*
-		 * We got the next subplan tuple successfully, so adjust state.
-		 */
-		if (ScanDirectionIsForward(direction))
-			limitstate->position++;
-		else
-		{
-			limitstate->position--;
-			Assert(limitstate->position > 0);
-		}
-		limitstate->atEnd = false;
-
-		/*
-		 * Now, is this a tuple we want?  If not, loop around to fetch
-		 * another tuple from the subplan.
-		 */
-		if (limitstate->position > limitstate->offset &&
-			(limitstate->noCount || limitstate->position <= netlimit))
+			break;
+
+		case LIMIT_SUBPLANEOF:
+			if (ScanDirectionIsForward(direction))
+				return NULL;
+			/*
+			 * Backing up from subplan EOF, so re-fetch previous tuple;
+			 * there should be one!  Note previous tuple must be in window.
+			 */
+			slot = ExecProcNode(outerPlan, (Plan *) node);
+			if (TupIsNull(slot))
+				elog(ERROR, "ExecLimit: subplan failed to run backwards");
+			limitstate->subSlot = slot;
+			limitstate->lstate = LIMIT_INWINDOW;
+			/* position does not change 'cause we didn't advance it before */
+			break;
+
+		case LIMIT_WINDOWEND:
+			if (ScanDirectionIsForward(direction))
+				return NULL;
+			/*
+			 * Backing up from window end: simply re-return the last
+			 * tuple fetched from the subplan.
+			 */
+			slot = limitstate->subSlot;
+			limitstate->lstate = LIMIT_INWINDOW;
+			/* position does not change 'cause we didn't advance it before */
+			break;
+
+		case LIMIT_WINDOWSTART:
+			if (!ScanDirectionIsForward(direction))
+				return NULL;
+			/*
+			 * Advancing after having backed off window start: simply
+			 * re-return the last tuple fetched from the subplan.
+			 */
+			slot = limitstate->subSlot;
+			limitstate->lstate = LIMIT_INWINDOW;
+			/* position does not change 'cause we didn't change it before */
+			break;
+
+		default:
+			elog(ERROR, "ExecLimit: impossible state %d",
+				 (int) limitstate->lstate);
+			slot = NULL;		/* keep compiler quiet */
 			break;
 	}
 
+	/* Return the current tuple */
+	Assert(!TupIsNull(slot));
+
 	ExecStoreTuple(slot->val,
 				   resultTupleSlot,
 				   InvalidBuffer,
@@ -181,6 +247,7 @@ recompute_limits(Limit *node)
 
 	if (node->limitCount)
 	{
+		limitstate->noCount = false;
 		limitstate->count =
 			DatumGetInt32(ExecEvalExprSwitchContext(node->limitCount,
 													econtext,
@@ -199,12 +266,9 @@ recompute_limits(Limit *node)
 		limitstate->noCount = true;
 	}
 
-	/* Reset position data to start-of-scan */
+	/* Reset position to start-of-scan */
 	limitstate->position = 0;
-	limitstate->atEnd = false;
-
-	/* Set flag that params are computed */
-	limitstate->parmsSet = true;
+	limitstate->subSlot = NULL;
 }
 
 /* ----------------------------------------------------------------
@@ -230,7 +294,7 @@ ExecInitLimit(Limit *node, EState *estate, Plan *parent)
 	 */
 	limitstate = makeNode(LimitState);
 	node->limitstate = limitstate;
-	limitstate->parmsSet = false;
+	limitstate->lstate = LIMIT_INITIAL;
 
 	/*
 	 * Miscellaneous initialization
@@ -297,10 +361,10 @@ ExecReScanLimit(Limit *node, ExprContext *exprCtxt, Plan *parent)
 {
 	LimitState *limitstate = node->limitstate;
 
-	ExecClearTuple(limitstate->cstate.cs_ResultTupleSlot);
+	/* resetting lstate will force offset/limit recalculation */
+	limitstate->lstate = LIMIT_INITIAL;
 
-	/* force recalculation of limit expressions on first call */
-	limitstate->parmsSet = false;
+	ExecClearTuple(limitstate->cstate.cs_ResultTupleSlot);
 
 	/*
 	 * if chgParam of subnode is not null then plan will be re-scanned by

src/include/nodes/execnodes.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: execnodes.h,v 1.78 2002/11/15 02:50:10 momjian Exp $
+ * $Id: execnodes.h,v 1.79 2002/11/22 22:10:01 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -775,17 +775,28 @@ typedef struct SetOpState
  * offset is the number of initial tuples to skip (0 does nothing).
  * count is the number of tuples to return after skipping the offset tuples.
  * If no limit count was specified, count is undefined and noCount is true.
+ * When lstate == LIMIT_INITIAL, offset/count/noCount haven't been set yet.
  * ----------------
  */
+typedef enum
+{
+	LIMIT_INITIAL,				/* initial state for LIMIT node */
+	LIMIT_EMPTY,				/* there are no returnable rows */
+	LIMIT_INWINDOW,				/* have returned a row in the window */
+	LIMIT_SUBPLANEOF,			/* at EOF of subplan (within window) */
+	LIMIT_WINDOWEND,			/* stepped off end of window */
+	LIMIT_WINDOWSTART			/* stepped off beginning of window */
+} LimitStateCond;
+
 typedef struct LimitState
 {
 	CommonState cstate;			/* its first field is NodeTag */
 	long		offset;			/* current OFFSET value */
 	long		count;			/* current COUNT, if any */
-	long		position;		/* 1-based index of last tuple fetched */
-	bool		parmsSet;		/* have we calculated offset/limit yet? */
 	bool		noCount;		/* if true, ignore count */
-	bool		atEnd;			/* if true, we've reached EOF of subplan */
+	LimitStateCond lstate;		/* state machine status, as above */
+	long		position;		/* 1-based index of last tuple returned */
+	TupleTableSlot *subSlot;	/* tuple last obtained from subplan */
 } LimitState;