Skip to content

Commit b11123b

Browse files
tglsfdctglsfdc
committed
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.
1 parent 6405842 commit b11123b

File tree

5 files changed

+63
-13
lines changed

5 files changed

+63
-13
lines changed

‎src/backend/executor/nodeLimit.c

+23-11
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.30 2007/05/04 01:13:43 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.31 2007/05/17 19:35:08 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -55,17 +55,22 @@ ExecLimit(LimitState *node)
5555
case LIMIT_INITIAL:
5656

5757
/*
58-
* If backwards scan, just return NULL without changing state.
58+
* First call for this node, so compute limit/offset. (We can't do
59+
* this any earlier, because parameters from upper nodes will not
60+
* be set during ExecInitLimit.) This also sets position = 0
61+
* and changes the state to LIMIT_RESCAN.
5962
*/
60-
if (!ScanDirectionIsForward(direction))
61-
return NULL;
63+
recompute_limits(node);
64+
65+
/* FALL THRU */
66+
67+
case LIMIT_RESCAN:
6268

6369
/*
64-
* First call for this scan, so compute limit/offset. (We can't do
65-
* this any earlier, because parameters from upper nodes may not
66-
* be set until now.) This also sets position = 0.
70+
* If backwards scan, just return NULL without changing state.
6771
*/
68-
recompute_limits(node);
72+
if (!ScanDirectionIsForward(direction))
73+
return NULL;
6974

7075
/*
7176
* Check for empty window; if so, treat like empty subplan.
@@ -217,7 +222,7 @@ ExecLimit(LimitState *node)
217222
}
218223

219224
/*
220-
* Evaluate the limit/offset expressions --- done at start of each scan.
225+
* Evaluate the limit/offset expressions --- done at startup or rescan.
221226
*
222227
* This is also a handy place to reset the current-position state info.
223228
*/
@@ -281,6 +286,9 @@ recompute_limits(LimitState *node)
281286
node->position = 0;
282287
node->subSlot = NULL;
283288

289+
/* Set state-machine state */
290+
node->lstate = LIMIT_RESCAN;
291+
284292
/*
285293
* If we have a COUNT, and our input is a Sort node, notify it that it can
286294
* use bounded sort.
@@ -403,8 +411,12 @@ ExecEndLimit(LimitState *node)
403411
void
404412
ExecReScanLimit(LimitState *node, ExprContext *exprCtxt)
405413
{
406-
/* resetting lstate will force offset/limit recalculation */
407-
node->lstate = LIMIT_INITIAL;
414+
/*
415+
* Recompute limit/offset in case parameters changed, and reset the
416+
* state machine. We must do this before rescanning our child node,
417+
* in case it's a Sort that we are passing the parameters down to.
418+
*/
419+
recompute_limits(node);
408420

409421
/*
410422
* if chgParam of subnode is not null then plan will be re-scanned by

‎src/backend/executor/nodeSubplan.c

+8-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.88 2007/04/26 23:24:44 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/executor/nodeSubplan.c,v 1.89 2007/05/17 19:35:08 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -242,6 +242,9 @@ ExecScanSubPlan(SubPlanState *node,
242242
planstate->chgParam = bms_add_member(planstate->chgParam, paramid);
243243
}
244244

245+
/*
246+
* Now that we've set up its parameters, we can reset the subplan.
247+
*/
245248
ExecReScan(planstate, NULL);
246249

247250
/*
@@ -901,6 +904,10 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
901904
subLinkType == ALL_SUBLINK)
902905
elog(ERROR, "ANY/ALL subselect unsupported as initplan");
903906

907+
/*
908+
* By definition, an initplan has no parameters from our query level,
909+
* but it could have some from an outer level. Rescan it if needed.
910+
*/
904911
if (planstate->chgParam != NULL)
905912
ExecReScan(planstate, NULL);
906913

‎src/include/nodes/execnodes.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.173 2007/05/04 01:13:45 tgl Exp $
10+
* $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.174 2007/05/17 19:35:08 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -1416,6 +1416,7 @@ typedef struct SetOpState
14161416
typedef enum
14171417
{
14181418
LIMIT_INITIAL, /* initial state for LIMIT node */
1419+
LIMIT_RESCAN, /* rescan after recomputing parameters */
14191420
LIMIT_EMPTY, /* there are no returnable rows */
14201421
LIMIT_INWINDOW, /* have returned a row in the window */
14211422
LIMIT_SUBPLANEOF, /* at EOF of subplan (within window) */

‎src/test/regress/expected/limit.out

+21
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,24 @@ SELECT ''::text AS five, unique1, unique2, stringu1
108108
| 904 | 793 | UIAAAA
109109
(5 rows)
110110

111+
-- Stress test for variable LIMIT in conjunction with bounded-heap sorting
112+
SELECT
113+
(SELECT n
114+
FROM (VALUES (1)) AS x,
115+
(SELECT n FROM generate_series(1,10) AS n
116+
ORDER BY n LIMIT 1 OFFSET s-1) AS y) AS z
117+
FROM generate_series(1,10) AS s;
118+
z
119+
----
120+
1
121+
2
122+
3
123+
4
124+
5
125+
6
126+
7
127+
8
128+
9
129+
10
130+
(10 rows)
131+

‎src/test/regress/sql/limit.sql

+9
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,12 @@ SELECT ''::text AS five, unique1, unique2, stringu1
3030
SELECT ''::text AS five, unique1, unique2, stringu1
3131
FROM onek
3232
ORDER BY unique1 LIMIT 5 OFFSET 900;
33+
34+
-- Stress test for variable LIMIT in conjunction with bounded-heap sorting
35+
36+
SELECT
37+
(SELECT n
38+
FROM (VALUES (1)) AS x,
39+
(SELECT n FROM generate_series(1,10) AS n
40+
ORDER BY n LIMIT 1 OFFSET s-1) AS y) AS z
41+
FROM generate_series(1,10) AS s;

0 commit comments

Comments
 (0)