On Fri, Feb 5, 2021 at 4:25 PM Hou, Zhijie <houzj.f...@cn.fujitsu.com> wrote:
>
> > >
> > > That is very close to what I was going to suggest, which is this:
> > >
> > > diff --git a/src/backend/rewrite/rewriteHandler.c
> > > b/src/backend/rewrite/rewriteHandler.c
> > > index 0672f497c6..3c4417af98 100644
> > > --- a/src/backend/rewrite/rewriteHandler.c
> > > +++ b/src/backend/rewrite/rewriteHandler.c
> > > @@ -631,6 +631,8 @@ rewriteRuleAction(Query *parsetree,
> > > checkExprHasSubLink((Node *)
> > > rule_action->returningList);
> > > }
> > >
> > > + rule_action->hasModifyingCTE |= parsetree->hasModifyingCTE;
> > > +
> > > return rule_action;
> > > }
> >
> >
> > if (parsetree->cteList != NIL && sub_action->commandType !=
> > CMD_UTILITY)
> > {
> > ...
> > sub_action->cteList = list_concat(sub_action->cteList,
> > }
> >
> > Is is possible when sub_action is CMD_UTILITY ?
> > In this case CTE will be copied to the newone, should we set the set the
> > flag in this case ?
>
> Sorry , a typo in my word.
> In this case CTE will not be copied to the newone, should we set the set the
> flag in this case ?
>
No, strictly speaking, we probably shouldn't, because the CTE wasn't
copied in that case.
Also, I know the bitwise OR "works" in this case, but I think some
will frown on use of that for a bool.
IMHO better to use:
if (parsetree->hasModifyingCTE)
rule_action->hasModifyingCTE = true;
So patch might be something like:
diff --git a/src/backend/rewrite/rewriteHandler.c
b/src/backend/rewrite/rewriteHandler.c
index 0672f497c6..a989e02925 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -557,6 +557,8 @@ 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;
}
/*
@@ -594,6 +596,9 @@ rewriteRuleAction(Query *parsetree,
*sub_action_ptr = sub_action;
else
rule_action = sub_action;
+
+ if (parsetree->hasModifyingCTE)
+ sub_action->hasModifyingCTE = true;
}
/*
I'll do some further checks, because the rewriting is recursive and
tricky, so don't want to miss any cases ...
Regards,
Greg Nancarrow
Fujitsu Australia
