Horton

Re: Table refer leak in logical replication

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Amit Langote <amitlangote09@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Michael Paquier <michael@paquier.xyz>
Date: 2021-04-20T12:50:03Z
Lists: pgsql-hackers 
On Tue, Apr 20, 2021 at 4:59 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> > > ... FWIW, I'd rather
> > > agree to use what has been proposed with es_opened_result_relations
> > > like TRUNCATE does rather than attempt to use ExecInitResultRelation()
> > > combined with potentially asymmetric calls to
> > > ExecCloseResultRelations().
> >
> > Okay, how about the attached then?  I decided to go with just finish_estate(),
> > because we no longer have to do anything relation specific there.
> >
>
> I think the patch looks good.
> But I noticed that there seems no testcase to test the [aftertrigger in subscriber] when using logical replication.
> As we seems planned to do some further refactor in the future, Is it better to add one testcase to cover this code ?
>

+1. I think it makes sense to add a test case especially because we
don't have any existing test in this area.


-- 
With Regards,
Amit Kapila.

Commits

  1. Add more tests with triggers on partitions for logical replication

  2. Fix relation leak for subscribers firing triggers in logical replication

  3. Create ResultRelInfos later in InitPlan, index them by RT index.