Skip to content

Commit caf97eb

Browse files
tglsfdctglsfdc
committed
Fix rescan logic in nodeCtescan.
The previous coding essentially assumed that nodes would be rescanned in the same order they were initialized in; or at least that the "leader" of a group of CTEscans would be rescanned before any others were required to execute. Unfortunately, that isn't even a little bit true. It's possible to devise queries in which the leader isn't rescanned until other CTEscans on the same CTE have run to completion, or even in which the leader never gets a rescan call at all. The fix makes the leader specially responsible only for initial creation and final destruction of the tuplestore; rescan resets are now a symmetrically shared responsibility. This means that we might reset the tuplestore multiple times when restarting a plan subtree containing multiple CTEscans; but resetting an already-empty tuplestore is cheap enough that that doesn't seem like a problem. Per report from Adam Mackler; the new regression test cases are based on his example query. Back-patch to 8.4 where CTE scans were introduced.
1 parent 82634a8 commit caf97eb

File tree

3 files changed

+151
-17
lines changed

3 files changed

+151
-17
lines changed

‎src/backend/executor/nodeCtescan.c

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ ExecInitCteScan(CteScan *node, EState *estate, int eflags)
205205
* The Param slot associated with the CTE query is used to hold a pointer
206206
* to the CteState of the first CteScan node that initializes for this
207207
* CTE. This node will be the one that holds the shared state for all the
208-
* CTEs.
208+
* CTEs, particularly the shared tuplestore.
209209
*/
210210
prmdata = &(estate->es_param_exec_vals[node->cteParam]);
211211
Assert(prmdata->execPlan == NULL);
@@ -294,7 +294,10 @@ ExecEndCteScan(CteScanState *node)
294294
* If I am the leader, free the tuplestore.
295295
*/
296296
if (node->leader == node)
297+
{
297298
tuplestore_end(node->cte_table);
299+
node->cte_table = NULL;
300+
}
298301
}
299302

300303
/* ----------------------------------------------------------------
@@ -312,26 +315,26 @@ ExecReScanCteScan(CteScanState *node)
312315

313316
ExecScanReScan(&node->ss);
314317

315-
if (node->leader == node)
318+
/*
319+
* Clear the tuplestore if a new scan of the underlying CTE is required.
320+
* This implicitly resets all the tuplestore's read pointers. Note that
321+
* multiple CTE nodes might redundantly clear the tuplestore; that's OK,
322+
* and not unduly expensive. We'll stop taking this path as soon as
323+
* somebody has attempted to read something from the underlying CTE
324+
* (thereby causing its chgParam to be cleared).
325+
*/
326+
if (node->leader->cteplanstate->chgParam != NULL)
316327
{
317-
/*
318-
* The leader is responsible for clearing the tuplestore if a new scan
319-
* of the underlying CTE is required.
320-
*/
321-
if (node->cteplanstate->chgParam != NULL)
322-
{
323-
tuplestore_clear(tuplestorestate);
324-
node->eof_cte = false;
325-
}
326-
else
327-
{
328-
tuplestore_select_read_pointer(tuplestorestate, node->readptr);
329-
tuplestore_rescan(tuplestorestate);
330-
}
328+
tuplestore_clear(tuplestorestate);
329+
node->leader->eof_cte = false;
331330
}
332331
else
333332
{
334-
/* Not leader, so just rewind my own pointer */
333+
/*
334+
* Else, just rewind my own pointer. Either the underlying CTE
335+
* doesn't need a rescan (and we can re-read what's in the tuplestore
336+
* now), or somebody else already took care of it.
337+
*/
335338
tuplestore_select_read_pointer(tuplestorestate, node->readptr);
336339
tuplestore_rescan(tuplestorestate);
337340
}

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,81 @@ SELECT * FROM outermost;
12111211
ERROR: recursive reference to query "outermost" must not appear within a subquery
12121212
LINE 2: WITH innermost as (SELECT 2 FROM outermost)
12131213
^
1214+
--
1215+
-- Test CTEs read in non-initialization orders
1216+
--
1217+
WITH RECURSIVE
1218+
tab(id_key,link) AS (VALUES (1,17), (2,17), (3,17), (4,17), (6,17), (5,17)),
1219+
iter (id_key, row_type, link) AS (
1220+
SELECT 0, 'base', 17
1221+
UNION ALL (
1222+
WITH remaining(id_key, row_type, link, min) AS (
1223+
SELECT tab.id_key, 'true'::text, iter.link, MIN(tab.id_key) OVER ()
1224+
FROM tab INNER JOIN iter USING (link)
1225+
WHERE tab.id_key > iter.id_key
1226+
),
1227+
first_remaining AS (
1228+
SELECT id_key, row_type, link
1229+
FROM remaining
1230+
WHERE id_key=min
1231+
),
1232+
effect AS (
1233+
SELECT tab.id_key, 'new'::text, tab.link
1234+
FROM first_remaining e INNER JOIN tab ON e.id_key=tab.id_key
1235+
WHERE e.row_type = 'false'
1236+
)
1237+
SELECT * FROM first_remaining
1238+
UNION ALL SELECT * FROM effect
1239+
)
1240+
)
1241+
SELECT * FROM iter;
1242+
id_key | row_type | link
1243+
--------+----------+------
1244+
0 | base | 17
1245+
1 | true | 17
1246+
2 | true | 17
1247+
3 | true | 17
1248+
4 | true | 17
1249+
5 | true | 17
1250+
6 | true | 17
1251+
(7 rows)
1252+
1253+
WITH RECURSIVE
1254+
tab(id_key,link) AS (VALUES (1,17), (2,17), (3,17), (4,17), (6,17), (5,17)),
1255+
iter (id_key, row_type, link) AS (
1256+
SELECT 0, 'base', 17
1257+
UNION (
1258+
WITH remaining(id_key, row_type, link, min) AS (
1259+
SELECT tab.id_key, 'true'::text, iter.link, MIN(tab.id_key) OVER ()
1260+
FROM tab INNER JOIN iter USING (link)
1261+
WHERE tab.id_key > iter.id_key
1262+
),
1263+
first_remaining AS (
1264+
SELECT id_key, row_type, link
1265+
FROM remaining
1266+
WHERE id_key=min
1267+
),
1268+
effect AS (
1269+
SELECT tab.id_key, 'new'::text, tab.link
1270+
FROM first_remaining e INNER JOIN tab ON e.id_key=tab.id_key
1271+
WHERE e.row_type = 'false'
1272+
)
1273+
SELECT * FROM first_remaining
1274+
UNION ALL SELECT * FROM effect
1275+
)
1276+
)
1277+
SELECT * FROM iter;
1278+
id_key | row_type | link
1279+
--------+----------+------
1280+
0 | base | 17
1281+
1 | true | 17
1282+
2 | true | 17
1283+
3 | true | 17
1284+
4 | true | 17
1285+
5 | true | 17
1286+
6 | true | 17
1287+
(7 rows)
1288+
12141289
--
12151290
-- Data-modifying statements in WITH
12161291
--

‎src/test/regress/sql/with.sql

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,62 @@ WITH RECURSIVE outermost(x) AS (
574574
)
575575
SELECT * FROM outermost;
576576

577+
--
578+
-- Test CTEs read in non-initialization orders
579+
--
580+
581+
WITH RECURSIVE
582+
tab(id_key,link) AS (VALUES (1,17), (2,17), (3,17), (4,17), (6,17), (5,17)),
583+
iter (id_key, row_type, link) AS (
584+
SELECT 0, 'base', 17
585+
UNION ALL (
586+
WITH remaining(id_key, row_type, link, min) AS (
587+
SELECT tab.id_key, 'true'::text, iter.link, MIN(tab.id_key) OVER ()
588+
FROM tab INNER JOIN iter USING (link)
589+
WHERE tab.id_key > iter.id_key
590+
),
591+
first_remaining AS (
592+
SELECT id_key, row_type, link
593+
FROM remaining
594+
WHERE id_key=min
595+
),
596+
effect AS (
597+
SELECT tab.id_key, 'new'::text, tab.link
598+
FROM first_remaining e INNER JOIN tab ON e.id_key=tab.id_key
599+
WHERE e.row_type = 'false'
600+
)
601+
SELECT * FROM first_remaining
602+
UNION ALL SELECT * FROM effect
603+
)
604+
)
605+
SELECT * FROM iter;
606+
607+
WITH RECURSIVE
608+
tab(id_key,link) AS (VALUES (1,17), (2,17), (3,17), (4,17), (6,17), (5,17)),
609+
iter (id_key, row_type, link) AS (
610+
SELECT 0, 'base', 17
611+
UNION (
612+
WITH remaining(id_key, row_type, link, min) AS (
613+
SELECT tab.id_key, 'true'::text, iter.link, MIN(tab.id_key) OVER ()
614+
FROM tab INNER JOIN iter USING (link)
615+
WHERE tab.id_key > iter.id_key
616+
),
617+
first_remaining AS (
618+
SELECT id_key, row_type, link
619+
FROM remaining
620+
WHERE id_key=min
621+
),
622+
effect AS (
623+
SELECT tab.id_key, 'new'::text, tab.link
624+
FROM first_remaining e INNER JOIN tab ON e.id_key=tab.id_key
625+
WHERE e.row_type = 'false'
626+
)
627+
SELECT * FROM first_remaining
628+
UNION ALL SELECT * FROM effect
629+
)
630+
)
631+
SELECT * FROM iter;
632+
577633
--
578634
-- Data-modifying statements in WITH
579635
--

0 commit comments

Comments
 (0)