From cb0dea0115dc108d2b5c7fb4bd6310c1d742cac0 Mon Sep 17 00:00:00 2001 From: Takayuki Tsunakawa Date: Thu, 20 May 2021 23:01:46 +0900 Subject: [PATCH v3] propagate CTE property flags in rewriter --- src/backend/rewrite/rewriteHandler.c | 14 ++++++++++---- src/test/regress/expected/with.out | 27 +++++++++++++++++++++++++++ src/test/regress/sql/with.sql | 16 ++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 88a9e95..09bd2f3 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -467,7 +467,7 @@ rewriteRuleAction(Query *parsetree, * this is a no-op because RLS conditions aren't added till later, but it * seems like good future-proofing to do this anyway.) */ - sub_action->hasRowSecurity |= parsetree->hasRowSecurity; + sub_action->hasRowSecurity = parsetree->hasRowSecurity; /* * Each rule action's jointree should be the main parsetree's jointree @@ -524,7 +524,7 @@ rewriteRuleAction(Query *parsetree, * If the original query has any CTEs, copy them into the rule action. But * we don't need them for a utility action. */ - if (parsetree->cteList != NIL && sub_action->commandType != CMD_UTILITY) + if (parsetree->cteList != NIL && rule_action->commandType != CMD_UTILITY) { ListCell *lc; @@ -535,13 +535,16 @@ 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) { CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc); ListCell *lc2; - foreach(lc2, sub_action->cteList) + foreach(lc2, rule_action->cteList) { CommonTableExpr *cte2 = (CommonTableExpr *) lfirst(lc2); @@ -554,8 +557,11 @@ rewriteRuleAction(Query *parsetree, } /* OK, it's safe to combine the CTE lists */ - sub_action->cteList = list_concat(sub_action->cteList, + rule_action->cteList = list_concat(rule_action->cteList, copyObject(parsetree->cteList)); + /* ... and don't forget about the associated flags */ + rule_action->hasRecursive = parsetree->hasRecursive; + rule_action->hasModifyingCTE = parsetree->hasModifyingCTE; } /* diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 584bdc6..a1c5844 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -2397,6 +2397,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 1a9bdc9..d9b4d06 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -1126,6 +1126,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 -- 2.10.1