Re: Bug in query rewriter - hasModifyingCTE not getting set

After poking around a bit more, I notice that the hasRecursive flag
really ought to get propagated as well, since that's also an attribute
of the CTE list.  That omission doesn't seem to have any ill effect
today, since nothing in planning or execution looks at that flag, but
someday it might.  So what I think we should do is as attached.
(I re-integrated your example into with.sql, too.)

Given the very limited time remaining before the release wrap, I'm
going to go ahead and push this.

            regards, tom lane

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 0672f497c6..ba6f8a0507 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -536,6 +536,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)
         {
@@ -557,6 +560,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 c519a61c4f..5e4ea29243 100644
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -2277,6 +2277,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 f4ba0d8e39..0ffa44afa7 100644
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -1070,6 +1070,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