Revert "Propagate CTE property flags when copying a CTE list into a rule."

tglsfdc · tglsfdc · commit d1d297985253 · 2021-02-07T12:54:08.000-05:00
This reverts commit ed29089 and equivalent back-branch commits. The issue is subtler than I thought, and it's far from new, so just before a release deadline is no time to be fooling with it. We'll consider what to do at a bit more leisure. 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
@@ -536,9 +536,6 @@ 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)
 		{
@@ -560,9 +557,6 @@ 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
@@ -2277,33 +2277,6 @@ 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
@@ -1070,22 +1070,6 @@ 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

Original file line number	Diff line number	Diff line change
`@@ -536,9 +536,6 @@ rewriteRuleAction(Query *parsetree,`
`536`	`536`	`*`
`537`	`537`	`* This could possibly be fixed by using some sort of internally`
`538`	`538`	`* 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.`
`542`	`539`	`*/`
`543`	`540`	`foreach(lc, parsetree->cteList)`
`544`	`541`	`{`
`@@ -560,9 +557,6 @@ rewriteRuleAction(Query *parsetree,`
`560`	`557`	`/* OK, it's safe to combine the CTE lists */`
`561`	`558`	`sub_action->cteList = list_concat(sub_action->cteList,`
`562`	`559`	`copyObject(parsetree->cteList));`
`563`		`- /* ... and don't forget about the associated flags */`
`564`		`- sub_action->hasRecursive \|= parsetree->hasRecursive;`
`565`		`- sub_action->hasModifyingCTE \|= parsetree->hasModifyingCTE;`
`566`	`560`	`}`
`567`	`561`
`568`	`562`	`/*`