On Fri, Mar 12, 2021 at 1:33 PM houzj.f...@fujitsu.com <houzj.f...@fujitsu.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.
