From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Cc: | Greg Nancarrow <gregn4422(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com> |
Subject: | Re: Parallel INSERT (INTO ... SELECT ...) |
Date: | 2021-03-12 11:36:34 |
Message-ID: | CAA4eK1KuaPBYp_b4+xiQdqmaVY2hENuaCvMOAo40Yfo7Br8eWA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Mar 12, 2021 at 1:33 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> > > The problem is that target_rel_trigger_max_parallel_hazard and its
> > > caller think they can use a relcache TriggerDesc field across other
> > > cache accesses, which they can't because the relcache doesn't
> > > guarantee that that won't move.
> > >
> > > One approach would be to add logic to RelationClearRelation similar to
> > > what it does for tupdescs, rules, etc, to avoid moving them when their
> > > contents haven't changed. But given that we've not needed that for
> > > the past several decades, I'm disinclined to add the overhead. I
> > > think this code ought to be adjusted to not make its own copy of the
> > > trigdesc pointer, but instead fetch it out of the relcache struct each
> > > time it is accessed. There's no real reason why
> > > target_rel_trigger_max_parallel_hazard shouldn't be passed the
> > > (stable) Relation pointer instead of just the trigdesc pointer.
> > >
> >
> > I have attached a patch to fix the issue, based on your suggestion (tested with
> > CLOBBER_CACHE_ALWAYS defined).
> >
> > > BTW, having special logic for FK triggers in
> > > target_rel_trigger_max_parallel_hazard seems quite loony to me.
> > > Why isn't that handled by setting appropriate proparallel values for
> > > those trigger functions?
> > >
> >
> > ... and also attached a patch to update the code for this issue.
> >
> > (2nd patch relies on application of the 1st patch)
> >
> > Thanks again for pointing out these problems.
>
> I have tested the triggerdesc bugfix patch with CLOBBER_CACHE_ALWAYS flag.
> It passed the testset where is fail in buildfarm (foreign_key, foreign_data).
>
Thanks for the patch and review. It looks good to me as well and
passes the tests (foreign_key, foreign_data) with CLOBBER_CACHE_ALWAYS
flag.
I'll review the second patch of Greg.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Walker | 2021-03-12 12:12:53 | 回复: unrecognized configuration parameter "plpgsql.check_asserts" |
Previous Message | Amit Kapila | 2021-03-12 11:34:46 | Re: [Patch] Optimize dropping of relation buffers using dlist |