Fix parameter recalculation for Limit nodes: during a ReScan call we must

recompute the limit/offset immediately, so that the updated values are
available when the child's ReScan function is invoked.  Add a regression
test for this, too.  Bug is new in HEAD (due to the bounded-sorting patch)
so no need for back-patch.

I did not do anything about merging this signaling with chgParam processing,
but if we were to do that we'd still need to compute the updated values
at this point rather than during the first ProcNode call.

Per observation and test case from Greg Stark, though I didn't use his patch.

diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index 9d40952647d7bd425a0b4092f4a6aa0c3a5508fb..76296cfd8779005e22484be174fc9aaed6f467e8 100644 (file)
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.30 2007/05/04 01:13:43 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.31 2007/05/17 19:35:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,17 +55,22 @@ ExecLimit(LimitState *node)
                case LIMIT_INITIAL:
 
                        /*
-                        * If backwards scan, just return NULL without changing state.
+                        * First call for this node, so compute limit/offset. (We can't do
+                        * this any earlier, because parameters from upper nodes will not
+                        * be set during ExecInitLimit.)  This also sets position = 0
+                        * and changes the state to LIMIT_RESCAN.
                         */
-                       if (!ScanDirectionIsForward(direction))
-                               return NULL;
+                       recompute_limits(node);
+
+                       /* FALL THRU */
+
+               case LIMIT_RESCAN:
 
                        /*
-                        * 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.
+                        * If backwards scan, just return NULL without changing state.
                         */
-                       recompute_limits(node);
+                       if (!ScanDirectionIsForward(direction))
+                               return NULL;
 
                        /*
                         * Check for empty window; if so, treat like empty subplan.
@@ -217,7 +222,7 @@ ExecLimit(LimitState *node)
 }
 
 /*
- * Evaluate the limit/offset expressions --- done at start of each scan.
+ * Evaluate the limit/offset expressions --- done at startup or rescan.
  *
  * This is also a handy place to reset the current-position state info.
  */
@@ -281,6 +286,9 @@ recompute_limits(LimitState *node)
        node->position = 0;
        node->subSlot = NULL;
 
+       /* Set state-machine state */
+       node->lstate = LIMIT_RESCAN;
+
        /*
         * If we have a COUNT, and our input is a Sort node, notify it that it can
         * use bounded sort.
@@ -403,8 +411,12 @@ ExecEndLimit(LimitState *node)
 void
 ExecReScanLimit(LimitState *node, ExprContext *exprCtxt)
 {
-       /* resetting lstate will force offset/limit recalculation */
-       node->lstate = LIMIT_INITIAL;
+       /*
+        * Recompute limit/offset in case parameters changed, and reset the
+        * state machine.  We must do this before rescanning our child node,
+        * in case it's a Sort that we are passing the parameters down to.
+        */
+       recompute_limits(node);
 
        /*
         * if chgParam of subnode is not null then plan will be re-scanned by

diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index a66f51c26a62607195bce70cdb64d2c262a50839..ac91ed1be64d752b6eafe06648fb1346de4f838a 100644 (file)
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.88 2007/04/26 23:24:44 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.89 2007/05/17 19:35:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -242,6 +242,9 @@ ExecScanSubPlan(SubPlanState *node,
                planstate->chgParam = bms_add_member(planstate->chgParam, paramid);
        }
 
+       /*
+        * Now that we've set up its parameters, we can reset the subplan.
+        */
        ExecReScan(planstate, NULL);
 
        /*
@@ -901,6 +904,10 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
                subLinkType == ALL_SUBLINK)
                elog(ERROR, "ANY/ALL subselect unsupported as initplan");
 
+       /*
+        * By definition, an initplan has no parameters from our query level,
+        * but it could have some from an outer level.  Rescan it if needed.
+        */
        if (planstate->chgParam != NULL)
                ExecReScan(planstate, NULL);
 

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 12219b883ebc8886bf04b606e069732250fafa58..6f80080a09f2905e8cdd715fb0447f2298c55629 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.173 2007/05/04 01:13:45 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.174 2007/05/17 19:35:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1416,6 +1416,7 @@ typedef struct SetOpState
 typedef enum
 {
        LIMIT_INITIAL,                          /* initial state for LIMIT node */
+       LIMIT_RESCAN,                           /* rescan after recomputing parameters */
        LIMIT_EMPTY,                            /* there are no returnable rows */
        LIMIT_INWINDOW,                         /* have returned a row in the window */
        LIMIT_SUBPLANEOF,                       /* at EOF of subplan (within window) */

diff --git a/src/test/regress/expected/limit.out b/src/test/regress/expected/limit.out
index f960958b2f6d5d0e9992a6c4810d4ee1d7f058b1..c33ebe039658f09117db865ab3cc0b09982f61f3 100644 (file)
--- a/src/test/regress/expected/limit.out
+++ b/src/test/regress/expected/limit.out
@@ -108,3 +108,24 @@ SELECT ''::text AS five, unique1, unique2, stringu1
       |     904 |     793 | UIAAAA
 (5 rows)
 
+-- Stress test for variable LIMIT in conjunction with bounded-heap sorting
+SELECT
+  (SELECT n
+     FROM (VALUES (1)) AS x,
+          (SELECT n FROM generate_series(1,10) AS n
+             ORDER BY n LIMIT 1 OFFSET s-1) AS y) AS z
+  FROM generate_series(1,10) AS s;
+ z  
+----
+  1
+  2
+  3
+  4
+  5
+  6
+  7
+  8
+  9
+ 10
+(10 rows)
+

diff --git a/src/test/regress/sql/limit.sql b/src/test/regress/sql/limit.sql
index c15a486aff9cdba7d1e7b504539b73e2aadbf883..3004550b6584fb536ae5f4ace355b4fffd31fc36 100644 (file)
--- a/src/test/regress/sql/limit.sql
+++ b/src/test/regress/sql/limit.sql
@@ -30,3 +30,12 @@ SELECT ''::text AS five, unique1, unique2, stringu1
 SELECT ''::text AS five, unique1, unique2, stringu1 
                FROM onek
                ORDER BY unique1 LIMIT 5 OFFSET 900;
+
+-- Stress test for variable LIMIT in conjunction with bounded-heap sorting
+
+SELECT
+  (SELECT n
+     FROM (VALUES (1)) AS x,
+          (SELECT n FROM generate_series(1,10) AS n
+             ORDER BY n LIMIT 1 OFFSET s-1) AS y) AS z
+  FROM generate_series(1,10) AS s;