Re: Replica Identity check of partition table on subscriber

2022-06-23 Thread Amit Kapila
On Wed, Jun 22, 2022 at 5:05 PM houzj.f...@fujitsu.com wrote: > > On Wednesday, June 22, 2022 7:06 PM Amit Kapila > wrote: > > > > On Wed, Jun 22, 2022 at 10:09 AM Amit Langote > > wrote: > > > > > > On Wed, Jun 22, 2022 at 12:02 PM houzj.f...@fujitsu.com > > > wrote: > > > > Since the patch

RE: Replica Identity check of partition table on subscriber

2022-06-22 Thread houzj.f...@fujitsu.com
On Wednesday, June 22, 2022 7:06 PM Amit Kapila wrote: > > On Wed, Jun 22, 2022 at 10:09 AM Amit Langote > wrote: > > > > On Wed, Jun 22, 2022 at 12:02 PM houzj.f...@fujitsu.com > > wrote: > > > Since the patch has been committed. Attach the last patch to fix the > memory leak. > > > > > > The

Re: Replica Identity check of partition table on subscriber

2022-06-22 Thread Amit Langote
On Wed, Jun 22, 2022 at 8:05 PM Amit Kapila wrote: > On Wed, Jun 22, 2022 at 10:09 AM Amit Langote wrote: > > On Wed, Jun 22, 2022 at 12:02 PM houzj.f...@fujitsu.com > > wrote: > > > Since the patch has been committed. Attach the last patch to fix the > > > memory leak. > > > > > > The bug

Re: Replica Identity check of partition table on subscriber

2022-06-22 Thread Amit Kapila
On Wed, Jun 22, 2022 at 10:09 AM Amit Langote wrote: > > On Wed, Jun 22, 2022 at 12:02 PM houzj.f...@fujitsu.com > wrote: > > Since the patch has been committed. Attach the last patch to fix the memory > > leak. > > > > The bug exists on PG10 ~ PG15(HEAD). > > > > For HEAD,PG14,PG13, to fix the

Re: Replica Identity check of partition table on subscriber

2022-06-21 Thread Amit Langote
Hi, On Wed, Jun 22, 2022 at 12:02 PM houzj.f...@fujitsu.com wrote: > Since the patch has been committed. Attach the last patch to fix the memory > leak. > > The bug exists on PG10 ~ PG15(HEAD). > > For HEAD,PG14,PG13, to fix the memory leak, I think we should use > free_attrmap instead of pfree

RE: Replica Identity check of partition table on subscriber

2022-06-21 Thread houzj.f...@fujitsu.com
On Tuesday, June 21, 2022 4:49 PM Amit Kapila > > On Tue, Jun 21, 2022 at 12:50 PM Amit Langote > wrote: > > > > On Tue, Jun 21, 2022 at 3:35 PM houzj.f...@fujitsu.com > > wrote: > > > > Attached a patch containing the above to consider as an alternative. > > > > Thanks, the patch looks good

Re: Replica Identity check of partition table on subscriber

2022-06-21 Thread Amit Langote
On Tue, Jun 21, 2022 at 5:08 PM houzj.f...@fujitsu.com wrote: > On Tuesday, June 21, 2022 3:21 PM Amit Langote > wrote: > > Thanks for the patch. > > > > I agree it's an old bug. A partition map entry's localrel may point > > to a stale Relation pointer, because once the caller had closed the

RE: Replica Identity check of partition table on subscriber

2022-06-21 Thread shiy.f...@fujitsu.com
On Tuesday, June 21, 2022 4:49 PM Amit Kapila wrote: > > On Tue, Jun 21, 2022 at 12:50 PM Amit Langote > wrote: > > > > On Tue, Jun 21, 2022 at 3:35 PM houzj.f...@fujitsu.com > > wrote: > > > > Attached a patch containing the above to consider as an alternative. > > > > Thanks, the patch

Re: Replica Identity check of partition table on subscriber

2022-06-21 Thread Amit Kapila
On Tue, Jun 21, 2022 at 12:50 PM Amit Langote wrote: > > On Tue, Jun 21, 2022 at 3:35 PM houzj.f...@fujitsu.com > wrote: > > Attached a patch containing the above to consider as an alternative. > Thanks, the patch looks good to me. I'll push this after doing some testing. -- With Regards,

RE: Replica Identity check of partition table on subscriber

2022-06-21 Thread houzj.f...@fujitsu.com
On Tuesday, June 21, 2022 3:21 PM Amit Langote wrote: > > On Tue, Jun 21, 2022 at 3:35 PM houzj.f...@fujitsu.com > wrote: > > On Tuesday, June 21, 2022 1:29 PM Amit Kapila : > > > After pushing this patch, buildfarm member prion has failed. > > > >

Re: Replica Identity check of partition table on subscriber

2022-06-21 Thread Amit Langote
On Tue, Jun 21, 2022 at 3:35 PM houzj.f...@fujitsu.com wrote: > On Tuesday, June 21, 2022 1:29 PM Amit Kapila : > > After pushing this patch, buildfarm member prion has failed. > > https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=prion=HE > > AD > > > > It seems to me that the problem

RE: Replica Identity check of partition table on subscriber

2022-06-21 Thread houzj.f...@fujitsu.com
On Tuesday, June 21, 2022 1:29 PM Amit Kapila : > > On Tue, Jun 21, 2022 at 8:02 AM Amit Kapila wrote: > > > > On Tue, Jun 21, 2022 at 7:49 AM Amit Langote > wrote: > > > > > > > > > I think it should spell out REPLICA IDENTITY explicitly to avoid the > > > commit being confused to have to do

Re: Replica Identity check of partition table on subscriber

2022-06-20 Thread Amit Kapila
On Tue, Jun 21, 2022 at 8:02 AM Amit Kapila wrote: > > On Tue, Jun 21, 2022 at 7:49 AM Amit Langote wrote: > > > > > > I think it should spell out REPLICA IDENTITY explicitly to avoid the > > commit being confused to have to do with "Referential Integrity > > checking". > > > > This makes sense.

Re: Replica Identity check of partition table on subscriber

2022-06-20 Thread Amit Kapila
On Tue, Jun 21, 2022 at 7:49 AM Amit Langote wrote: > > On Mon, Jun 20, 2022 at 3:46 PM shiy.f...@fujitsu.com > wrote: > > On Mon, Jun 20, 2022 1:33 PM Amit Kapila wrote: > > > One minor comment: > > > + /* > > > + * If it is a partitioned table, we don't check it, we will check its > > > + *

Re: Replica Identity check of partition table on subscriber

2022-06-20 Thread Amit Langote
On Mon, Jun 20, 2022 at 3:46 PM shiy.f...@fujitsu.com wrote: > On Mon, Jun 20, 2022 1:33 PM Amit Kapila wrote: > > One minor comment: > > + /* > > + * If it is a partitioned table, we don't check it, we will check its > > + * partition later. > > + */ > > > > Can we change the above comment to:

RE: Replica Identity check of partition table on subscriber

2022-06-20 Thread shiy.f...@fujitsu.com
On Mon, Jun 20, 2022 1:33 PM Amit Kapila wrote: > > On Fri, Jun 17, 2022 at 11:22 AM shiy.f...@fujitsu.com > wrote: > > > > On Fri Jun 17, 2022 11:06 AM shiy.f...@fujitsu.com > wrote: > > > > > > Attached the new version of patch set. I also moved the partitioned table > > > check > > > in

Re: Replica Identity check of partition table on subscriber

2022-06-19 Thread Amit Kapila
On Fri, Jun 17, 2022 at 11:22 AM shiy.f...@fujitsu.com wrote: > > On Fri Jun 17, 2022 11:06 AM shiy.f...@fujitsu.com > wrote: > > > > Attached the new version of patch set. I also moved the partitioned table > > check > > in logicalrep_rel_mark_updatable() to check_relation_updatable() as > >

RE: Replica Identity check of partition table on subscriber

2022-06-16 Thread shiy.f...@fujitsu.com
On Fri Jun 17, 2022 11:06 AM shiy.f...@fujitsu.com wrote: > > Attached the new version of patch set. I also moved the partitioned table > check > in logicalrep_rel_mark_updatable() to check_relation_updatable() as > discussed > [2]. > Attached back-branch patches of the first patch. Regards,

RE: Replica Identity check of partition table on subscriber

2022-06-16 Thread shiy.f...@fujitsu.com
On Thu, Jun 16, 2022 2:13 PM Amit Langote wrote: > > Hi, > > On Thu, Jun 16, 2022 at 2:07 PM shiy.f...@fujitsu.com > wrote: > > On Wed, Jun 15, 2022 8:30 PM Amit Kapila > wrote: > > > I have pushed the first bug-fix patch today. > > > > Attached the remaining patches which are rebased. > >

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Langote
On Thu, Jun 16, 2022 at 9:28 PM Amit Kapila wrote: > On Thu, Jun 16, 2022 at 5:24 PM Amit Langote wrote: > > On Thu, Jun 16, 2022 at 6:42 PM Amit Kapila wrote: > > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > > wrote: > > > > @@ -1735,6 +1735,13 @@

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Kapila
On Thu, Jun 16, 2022 at 5:24 PM Amit Langote wrote: > > On Thu, Jun 16, 2022 at 6:42 PM Amit Kapila wrote: > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > wrote: > > > @@ -1735,6 +1735,13 @@ apply_handle_insert_internal(ApplyExecutionData > > > *edata, > > > static void > > >

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Langote
On Thu, Jun 16, 2022 at 6:42 PM Amit Kapila wrote: > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote wrote: > > @@ -1735,6 +1735,13 @@ apply_handle_insert_internal(ApplyExecutionData > > *edata, > > static void > > check_relation_updatable(LogicalRepRelMapEntry *rel) > > { > > + /* > > +*

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Kapila
On Fri, Jun 10, 2022 at 2:26 PM Amit Langote wrote: > > @@ -1735,6 +1735,13 @@ apply_handle_insert_internal(ApplyExecutionData *edata, > static void > check_relation_updatable(LogicalRepRelMapEntry *rel) > { > + /* > +* If it is a partitioned table, we don't check it, we will check its >

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Kapila
On Thu, Jun 16, 2022 at 12:30 PM Amit Langote wrote: > > On Thu, Jun 16, 2022 at 3:45 PM Amit Kapila wrote: > > On Thu, Jun 16, 2022 at 11:43 AM Amit Langote > > wrote: > > > + * Don't throw any error here just mark the relation entry as not > > > updatable, > > > + * as replica identity is

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Langote
On Thu, Jun 16, 2022 at 3:45 PM Amit Kapila wrote: > On Thu, Jun 16, 2022 at 11:43 AM Amit Langote wrote: > > + * Don't throw any error here just mark the relation entry as not > > updatable, > > + * as replica identity is only for updates and deletes but inserts can be > > + * replicated even

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Kapila
On Thu, Jun 16, 2022 at 11:43 AM Amit Langote wrote: > > On Thu, Jun 16, 2022 at 2:07 PM shiy.f...@fujitsu.com > wrote: > > On Wed, Jun 15, 2022 8:30 PM Amit Kapila wrote: > > > I have pushed the first bug-fix patch today. > > > > Attached the remaining patches which are rebased. > > Thanks. >

Re: Replica Identity check of partition table on subscriber

2022-06-16 Thread Amit Langote
Hi, On Thu, Jun 16, 2022 at 2:07 PM shiy.f...@fujitsu.com wrote: > On Wed, Jun 15, 2022 8:30 PM Amit Kapila wrote: > > I have pushed the first bug-fix patch today. > > Attached the remaining patches which are rebased. Thanks. Comments on v9-0001: + * Don't throw any error here just mark the

RE: Replica Identity check of partition table on subscriber

2022-06-15 Thread shiy.f...@fujitsu.com
On Wed, Jun 15, 2022 8:30 PM Amit Kapila wrote: > > I have pushed the first bug-fix patch today. > Thanks. Attached the remaining patches which are rebased. Regards, Shi yu v9-0002-fix-memory-leak-about-attrmap.patch Description: v9-0002-fix-memory-leak-about-attrmap.patch

Re: Replica Identity check of partition table on subscriber

2022-06-15 Thread Amit Kapila
On Wed, Jun 15, 2022 at 8:52 AM shiy.f...@fujitsu.com wrote: > > On Tue, Jun 14, 2022 8:57 PM Amit Kapila wrote: > > > > > v4-0002 looks good btw, except the bitpick about test comment similar > > > to my earlier comment regarding v5-0001: > > > > > > +# Change the column order of table on

RE: Replica Identity check of partition table on subscriber

2022-06-14 Thread shiy.f...@fujitsu.com
On Tue, Jun 14, 2022 8:57 PM Amit Kapila wrote: > > > v4-0002 looks good btw, except the bitpick about test comment similar > > to my earlier comment regarding v5-0001: > > > > +# Change the column order of table on publisher > > > > I think it might be better to say something specific to

Re: Replica Identity check of partition table on subscriber

2022-06-14 Thread Amit Langote
On Tue, Jun 14, 2022 at 9:57 PM Amit Kapila wrote: > On Tue, Jun 14, 2022 at 1:02 PM Amit Langote wrote: > > > +# Change the column order of table on publisher > > I think it might be better to say something specific to describe the > > test intent, like: > > > > Test that replication into

Re: Replica Identity check of partition table on subscriber

2022-06-14 Thread Amit Kapila
On Tue, Jun 14, 2022 at 1:02 PM Amit Langote wrote: > > On Tue, Jun 14, 2022 at 3:31 PM Amit Langote wrote: > > On Mon, Jun 13, 2022 at 6:14 PM Amit Kapila wrote: > > > I think we can do that way as well but do you see any benefit in it? > > > The way I am suggesting will avoid the effort of

RE: Replica Identity check of partition table on subscriber

2022-06-14 Thread shiy.f...@fujitsu.com
On Tue, Jun 14, 2022 2:18 PM Amit Langote wrote: > > On Mon, Jun 13, 2022 at 9:26 PM Amit Kapila > wrote: > > On Mon, Jun 13, 2022 at 1:03 PM houzj.f...@fujitsu.com > > wrote: > > > On Monday, June 13, 2022 1:53 PM Amit Kapila > wrote: > > > I have separated out the bug-fix for the

Re: Replica Identity check of partition table on subscriber

2022-06-14 Thread Amit Langote
On Tue, Jun 14, 2022 at 3:31 PM Amit Langote wrote: > On Mon, Jun 13, 2022 at 6:14 PM Amit Kapila wrote: > > I think we can do that way as well but do you see any benefit in it? > > The way I am suggesting will avoid the effort of updating the remote > > rel copy till we try to access that

Re: Replica Identity check of partition table on subscriber

2022-06-14 Thread Amit Langote
On Mon, Jun 13, 2022 at 6:14 PM Amit Kapila wrote: > On Mon, Jun 13, 2022 at 2:20 PM Amit Langote wrote: > > On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila > > wrote: > > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > > wrote: > > > > > > > > +logicalrep_partmap_invalidate > > > > > > > >

Re: Replica Identity check of partition table on subscriber

2022-06-14 Thread Amit Langote
On Mon, Jun 13, 2022 at 9:26 PM Amit Kapila wrote: > On Mon, Jun 13, 2022 at 1:03 PM houzj.f...@fujitsu.com > wrote: > > On Monday, June 13, 2022 1:53 PM Amit Kapila > > wrote: > > I have separated out the bug-fix for the subscriber-side. > > And fix the typo and function name. > > Attach the

Re: Replica Identity check of partition table on subscriber

2022-06-13 Thread Amit Kapila
On Mon, Jun 13, 2022 at 1:03 PM houzj.f...@fujitsu.com wrote: > > On Monday, June 13, 2022 1:53 PM Amit Kapila wrote: > > I have separated out the bug-fix for the subscriber-side. > And fix the typo and function name. > Attach the new version patch set. > The first patch looks good to me. I

Re: Replica Identity check of partition table on subscriber

2022-06-13 Thread Amit Kapila
On Mon, Jun 13, 2022 at 2:20 PM Amit Langote wrote: > > On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila wrote: > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > wrote: > > > > > > +logicalrep_partmap_invalidate > > > > > > I wonder why not call this logicalrep_partmap_update() to go with > > >

Re: Replica Identity check of partition table on subscriber

2022-06-13 Thread Amit Langote
On Sat, Jun 11, 2022 at 10:36 AM Amit Kapila wrote: > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote wrote: > > > > +logicalrep_partmap_invalidate > > > > I wonder why not call this logicalrep_partmap_update() to go with > > logicalrep_relmap_update()? It seems confusing to have > >

RE: Replica Identity check of partition table on subscriber

2022-06-13 Thread houzj.f...@fujitsu.com
On Monday, June 13, 2022 1:53 PM Amit Kapila wrote: > > On Sat, Jun 11, 2022 at 2:36 PM houzj.f...@fujitsu.com > wrote: > > > > On Saturday, June 11, 2022 9:36 AM Amit Kapila > wrote: > > > > > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > > > > > wrote: > > > > > > > >

Re: Replica Identity check of partition table on subscriber

2022-06-12 Thread Amit Kapila
On Sat, Jun 11, 2022 at 2:36 PM houzj.f...@fujitsu.com wrote: > > On Saturday, June 11, 2022 9:36 AM Amit Kapila > wrote: > > > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > > wrote: > > > > > > +logicalrep_partmap_invalidate > > > > > > I wonder why not call this

RE: Replica Identity check of partition table on subscriber

2022-06-11 Thread houzj.f...@fujitsu.com
On Saturday, June 11, 2022 9:36 AM Amit Kapila wrote: > > On Fri, Jun 10, 2022 at 2:26 PM Amit Langote > wrote: > > > > +logicalrep_partmap_invalidate > > > > I wonder why not call this logicalrep_partmap_update() to go with > > logicalrep_relmap_update()? It seems confusing to have > >

Re: Replica Identity check of partition table on subscriber

2022-06-10 Thread Amit Kapila
On Fri, Jun 10, 2022 at 2:26 PM Amit Langote wrote: > > +logicalrep_partmap_invalidate > > I wonder why not call this logicalrep_partmap_update() to go with > logicalrep_relmap_update()? It seems confusing to have > logicalrep_partmap_invalidate() right next to >

Re: Replica Identity check of partition table on subscriber

2022-06-10 Thread Amit Langote
Hello, On Wed, Jun 8, 2022 at 5:47 PM shiy.f...@fujitsu.com wrote: > Hi hackers, > > I saw a problem in logical replication, when the target table on subscriber > is a > partitioned table, it only checks whether the Replica Identity of partitioned > table is consistent with the publisher, and

RE: Replica Identity check of partition table on subscriber

2022-06-10 Thread shiy.f...@fujitsu.com
On Thu, June 9, 2022 7:02 PM Amit Kapila wrote: > > > I think one approach to fix it is to check the target partition in this > > case, > > instead of the partitioned table. > > > > This approach sounds reasonable to me. One minor point: > +/* > + * Check that replica identity matches. > + * >

Re: Replica Identity check of partition table on subscriber

2022-06-09 Thread Amit Langote
Hi Amit, On Thu, Jun 9, 2022 at 8:02 PM Amit Kapila wrote: > On Wed, Jun 8, 2022 at 2:17 PM shiy.f...@fujitsu.com > wrote: > > I saw a problem in logical replication, when the target table on subscriber > > is a > > partitioned table, it only checks whether the Replica Identity of > >

Re: Replica Identity check of partition table on subscriber

2022-06-09 Thread Amit Kapila
On Wed, Jun 8, 2022 at 2:17 PM shiy.f...@fujitsu.com wrote: > > I saw a problem in logical replication, when the target table on subscriber > is a > partitioned table, it only checks whether the Replica Identity of partitioned > table is consistent with the publisher, and doesn't check Replica