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

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index d54fdd27fa0518d12701bf78dd2be6d70c639b77..4373b95862f1472a778bc408641be759aacf4e21 100644 (file)
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -526,6 +526,9 @@ rewriteRuleAction(Query *parsetree,
                 *
                 * This could possibly be fixed by using some sort of internally
                 * generated ID, instead of names, to link CTE RTEs to their CTEs.
+                * However, decompiling the results would be quite confusing; note the
+                * merge of hasRecursive flags below, which could change the apparent
+                * semantics of such redundantly-named CTEs.
                 */
                foreach(lc, parsetree->cteList)
                {
@@ -547,6 +550,9 @@ rewriteRuleAction(Query *parsetree,
                /* OK, it's safe to combine the CTE lists */
                sub_action->cteList = list_concat(sub_action->cteList,
                                                                                  copyObject(parsetree->cteList));
+               /* ... and don't forget about the associated flags */
+               sub_action->hasRecursive |= parsetree->hasRecursive;
+               sub_action->hasModifyingCTE |= parsetree->hasModifyingCTE;
        }
 
        /*

diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index 0646451fb0bc9b54765dad6b38d6522ed857ed5d..4f293c4c19419a4b5846e4551c2064f1402a73dd 100644 (file)
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1672,6 +1672,33 @@ SELECT * FROM bug6051_2;
  3
 (3 rows)
 
+-- silly example to verify that hasModifyingCTE flag is propagated
+CREATE TEMP TABLE bug6051_3 AS
+  select a from generate_series(11,13) as a;
+CREATE RULE bug6051_3_ins AS ON INSERT TO bug6051_3 DO INSTEAD
+  SELECT i FROM bug6051_2;
+BEGIN; SET LOCAL force_parallel_mode = on;
+WITH t1 AS ( DELETE FROM bug6051_3 RETURNING * )
+  INSERT INTO bug6051_3 SELECT * FROM t1;
+ i 
+---
+ 1
+ 2
+ 3
+ 1
+ 2
+ 3
+ 1
+ 2
+ 3
+(9 rows)
+
+COMMIT;
+SELECT * FROM bug6051_3;
+ a 
+---
+(0 rows)
+
 -- a truly recursive CTE in the same list
 WITH RECURSIVE t(a) AS (
        SELECT 0

diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index 512ff1b3ba20c46e456e423203d587254e4ceb1e..47159e57f70387d886dc53a82ffdd10beea2a65e 100644 (file)
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -773,6 +773,22 @@ INSERT INTO bug6051 SELECT * FROM t1;
 SELECT * FROM bug6051;
 SELECT * FROM bug6051_2;
 
+-- silly example to verify that hasModifyingCTE flag is propagated
+CREATE TEMP TABLE bug6051_3 AS
+  select a from generate_series(11,13) as a;
+
+CREATE RULE bug6051_3_ins AS ON INSERT TO bug6051_3 DO INSTEAD
+  SELECT i FROM bug6051_2;
+
+BEGIN; SET LOCAL force_parallel_mode = on;
+
+WITH t1 AS ( DELETE FROM bug6051_3 RETURNING * )
+  INSERT INTO bug6051_3 SELECT * FROM t1;
+
+COMMIT;
+
+SELECT * FROM bug6051_3;
+
 -- a truly recursive CTE in the same list
 WITH RECURSIVE t(a) AS (
        SELECT 0