On Fri, Feb 5, 2021 at 8:07 PM Amit Langote <amitlangot...@gmail.com> wrote:
>
>
> > This is one reason for my original approach (though I admit, it was
> > not optimal) because at least it was reliable and detected the
> > modifyingCTE after all the rewriting and kludgy code had finished.
>
> Yeah it's hard to go through all of this highly recursive legacy code
> to be sure that hasModifyingCTE is consistent with reality in *all*
> cases, but let's try to do it. No other has* flags are set
> after-the-fact, so I wouldn't bet on a committer letting this one
> through.
>
I have debugged the code a bit more now, and the following patch seems
to correctly fix the issue, at least for the known test cases.
(i.e. SELECT case, shared by houzj, and the INSERT...SELECT case, as
in the "with" regression tests, for which I originally detected the
issue)
diff --git a/src/backend/rewrite/rewriteHandler.c
b/src/backend/rewrite/rewriteHandler.c
index 0672f497c6..8f695b32ec 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -557,6 +557,12 @@ rewriteRuleAction(Query *parsetree,
/* OK, it's safe to combine the CTE lists */
sub_action->cteList = list_concat(sub_action->cteList,
copyObject(parsetree->cteList));
+ if (parsetree->hasModifyingCTE)
+ {
+ sub_action->hasModifyingCTE = true;
+ if (sub_action_ptr)
+ rule_action->hasModifyingCTE = true;
+ }
}
/*
Regards,
Greg Nancarrow
Fujitsu Australia
