Re: Parallel INSERT (INTO ... SELECT ...)

From: Greg Nancarrow <gregn4422(at)gmail(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)fujitsu(dot)com>, "Tang, Haiying" <tanghy(dot)fnst(at)cn(dot)fujitsu(dot)com>
Subject: Re: Parallel INSERT (INTO ... SELECT ...)
Date: 2021-02-05 09:55:49
Message-ID: CAJcOf-fUGM7xr9fZoV9=PKUOuPMyRsJGGPeQqAK+BLa7-ki=ww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Feb 5, 2021 at 8:07 PM Amit Langote <amitlangote09(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ajin Cherian 2021-02-05 10:01:04 Re: Single transaction in the tablesync worker?
Previous Message Bharath Rupireddy 2021-02-05 09:50:35 Re: Support ALTER SUBSCRIPTION ... ADD/DROP PUBLICATION ... syntax