On Fri, Feb 5, 2021 at 11:12 PM Amit Langote <amitlangot...@gmail.com> wrote:
>
>
> That seems good enough as far as I am concerned. Although either an
> Assert as follows or a comment why the if (sub_action_ptr) is needed
> seems warranted.
>
> if (sub_action_ptr)
> rule_action->hasModifyingCTE = true;
> else
> Assert(sub_action == rule_action);
>
> Does the Assert seem overly confident?
>
No, the Assert is exactly right, and I'll add a comment too.
See below.
I'll post the patch separately, if you can't see any further issues.
diff --git a/src/backend/rewrite/rewriteHandler.c
b/src/backend/rewrite/rewriteHandler.c
index 0672f497c6..05b80bd347 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -557,6 +557,21 @@ rewriteRuleAction(Query *parsetree,
/* OK, it's safe to combine the CTE lists */
sub_action->cteList = list_concat(sub_action->cteList,
copyObject(parsetree->cteList));
+
+ /*
+ * If the hasModifyingCTE flag is set in the source parsetree from
+ * which the CTE list is copied, the flag needs to be set in the
+ * sub_action and, if applicable, in the rule_action (INSERT...SELECT
+ * case).
+ */
+ if (parsetree->hasModifyingCTE)
+ {
+ sub_action->hasModifyingCTE = true;
+ if (sub_action_ptr)
+ rule_action->hasModifyingCTE = true;
+ else
+ Assert(sub_action == rule_action);
+ }
}
/*
Regards,
Greg Nancarrow
Fujitsu Australia
