Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ed29089

Browse files
committed
Propagate CTE property flags when copying a CTE list into a rule.
rewriteRuleAction() neglected this step, although it was careful to propagate other similar flags such as hasSubLinks or hasRowSecurity. Omitting to transfer hasRecursive is just cosmetic at the moment, but omitting hasModifyingCTE is a live bug, since the executor certainly looks at that. The proposed test case only fails back to v10, but since the executor examines hasModifyingCTE in 9.x as well, I suspect that a test case could be devised that fails in older branches. Given the nearness of the release deadline, though, I'm not going to spend time looking for a better test. Report and patch by Greg Nancarrow, cosmetic changes by me Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
1 parent dd705a0 commit ed29089

File tree

3 files changed

+49
-0
lines changed

3 files changed

+49
-0
lines changed

src/backend/rewrite/rewriteHandler.c

+6
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,9 @@ rewriteRuleAction(Query *parsetree,
536536
*
537537
* This could possibly be fixed by using some sort of internally
538538
* generated ID, instead of names, to link CTE RTEs to their CTEs.
539+
* However, decompiling the results would be quite confusing; note the
540+
* merge of hasRecursive flags below, which could change the apparent
541+
* semantics of such redundantly-named CTEs.
539542
*/
540543
foreach(lc, parsetree->cteList)
541544
{
@@ -557,6 +560,9 @@ rewriteRuleAction(Query *parsetree,
557560
/* OK, it's safe to combine the CTE lists */
558561
sub_action->cteList = list_concat(sub_action->cteList,
559562
copyObject(parsetree->cteList));
563+
/* ... and don't forget about the associated flags */
564+
sub_action->hasRecursive |= parsetree->hasRecursive;
565+
sub_action->hasModifyingCTE |= parsetree->hasModifyingCTE;
560566
}
561567

562568
/*

src/test/regress/expected/with.out

+27
Original file line numberDiff line numberDiff line change
@@ -2277,6 +2277,33 @@ SELECT * FROM bug6051_2;
22772277
3
22782278
(3 rows)
22792279

2280+
-- silly example to verify that hasModifyingCTE flag is propagated
2281+
CREATE TEMP TABLE bug6051_3 AS
2282+
select a from generate_series(11,13) as a;
2283+
CREATE RULE bug6051_3_ins AS ON INSERT TO bug6051_3 DO INSTEAD
2284+
SELECT i FROM bug6051_2;
2285+
BEGIN; SET LOCAL force_parallel_mode = on;
2286+
WITH t1 AS ( DELETE FROM bug6051_3 RETURNING * )
2287+
INSERT INTO bug6051_3 SELECT * FROM t1;
2288+
i
2289+
---
2290+
1
2291+
2
2292+
3
2293+
1
2294+
2
2295+
3
2296+
1
2297+
2
2298+
3
2299+
(9 rows)
2300+
2301+
COMMIT;
2302+
SELECT * FROM bug6051_3;
2303+
a
2304+
---
2305+
(0 rows)
2306+
22802307
-- a truly recursive CTE in the same list
22812308
WITH RECURSIVE t(a) AS (
22822309
SELECT 0

src/test/regress/sql/with.sql

+16
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,22 @@ INSERT INTO bug6051 SELECT * FROM t1;
10701070
SELECT * FROM bug6051;
10711071
SELECT * FROM bug6051_2;
10721072

1073+
-- silly example to verify that hasModifyingCTE flag is propagated
1074+
CREATE TEMP TABLE bug6051_3 AS
1075+
select a from generate_series(11,13) as a;
1076+
1077+
CREATE RULE bug6051_3_ins AS ON INSERT TO bug6051_3 DO INSTEAD
1078+
SELECT i FROM bug6051_2;
1079+
1080+
BEGIN; SET LOCAL force_parallel_mode = on;
1081+
1082+
WITH t1 AS ( DELETE FROM bug6051_3 RETURNING * )
1083+
INSERT INTO bug6051_3 SELECT * FROM t1;
1084+
1085+
COMMIT;
1086+
1087+
SELECT * FROM bug6051_3;
1088+
10731089
-- a truly recursive CTE in the same list
10741090
WITH RECURSIVE t(a) AS (
10751091
SELECT 0

0 commit comments

Comments
 (0)