public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: [email protected] <[email protected]>
To: [email protected] <[email protected]>
To: Amit Kapila <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Amit Langote <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: Greg Nancarrow <[email protected]>
Cc: vignesh C <[email protected]>
Subject: RE: Data is copied twice when specifying both child and parent table in publication
Date: Thu, 12 May 2022 01:47:32 +0000
Message-ID: <TYCPR01MB8373BEA17F82CBFC5121AF65EDCB9@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYCPR01MB837349ADD6D0DEEC758CD444EDC89@TYCPR01MB8373.jpnprd01.prod.outlook.com>
References: <CAA4eK1+RyFHLv1K+3qCdRmqEO0bz68Ga=Tzomvq4Rc40HnNO8A@mail.gmail.com>
<CAA4eK1Jy126np5v8wc-k-MTbnneYDhXrjdC0oNriU8ibGcMdGg@mail.gmail.com>
<OS0PR01MB5716DC2982CC735FDE388804940B9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS3PR01MB6275CD6236DEB022C0785E9B9EE69@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<OS0PR01MB5716546D48042D44C3F0675794F29@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OSZPR01MB63109BE4F5521C542A75AC9BFDF29@OSZPR01MB6310.jpnprd01.prod.outlook.com>
<CAA4eK1LWYqCyDM1_6wZETS0fZ79u+ciU8MAB3eu+R9Keg96rUg@mail.gmail.com>
<OS3PR01MB6275CC45E9BC9297261865559EF99@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<OSZPR01MB631098CBD1208A4B5A2241E1FDFD9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
<OS3PR01MB62751C2FA25E23E24AB6980B9EC69@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<TYCPR01MB837349ADD6D0DEEC758CD444EDC89@TYCPR01MB8373.jpnprd01.prod.outlook.com>
On Wednesday, May 11, 2022 11:33 AM I wrote:
> On Monday, May 9, 2022 10:51 AM [email protected]
> <[email protected]> wrote:
> > Attach new patches.
> > The patch for HEAD:
> > 1. Modify the approach. Enhance the API of function
> > pg_get_publication_tables to handle one publication or an array of
> > publications.
> > The patch for REL14:
> > 1. Improve the table sync SQL. [suggestions by Shi yu]
> Hi, thank you for updating the patch !
>
> Minor comments on your patch for HEAD v2.
>
> (1) commit message sentence
>
> I suggest below sentence.
>
> Kindly change from
> "... when subscribing to both publications using one subscription, the data is
> replicated twice in inital copy"
> to "subscribing to both publications from one subscription causes initial copy
> twice".
>
> (2) unused variable
>
> pg_publication.c: In function ‘pg_get_publication_tables’:
> pg_publication.c:1091:11: warning: unused variable ‘pubname’
> [-Wunused-variable]
> char *pubname;
>
> We can remove this.
>
> (3) free of allocated memory
>
> In the pg_get_publication_tables(),
> we don't free 'elems'. Don't we need it ?
>
> (4) some coding alignments
>
> 4-1.
>
> + List *tables_viaroot = NIL,
> ...
> + *current_table = NIL;
>
> I suggest we can put some variables
> into the condition for the first time call of this function, like tables_viaroot and
> current_table.
> When you agree, kindly change it.
>
> 4-2.
>
> + /*
> + * Publications support partitioned tables, although
> all changes
> + * are replicated using leaf partition identity and
> schema, so we
> + * only need those.
> + */
> + if (publication->alltables)
> + {
> + current_table =
> GetAllTablesPublicationRelations(publication->pubviaroot);
> + }
>
> This is not related to the change itself and now we are inheriting the previous
> curly brackets, but I think there's no harm in removing it, since it's only for one
> statement.
Hi,
One more thing I'd like to add is that
we don't hit the below code by tests.
In the HEAD v2, we add a new filtering logic in pg_get_publication_tables.
Although I'm not sure if this is related to the bug fix itself,
when we want to include it in this patch, then
I feel it's better to add some simple test for this part,
to cover all the new main paths and check if
new logic works correctly.
+ /*
+ * If a partition table is published in a publication with viaroot,
+ * and its parent or child table is published in another publication
+ * without viaroot. Then we need to move the parent or child table
+ * from tables to tables_viaroot.
+ *
+ * If all publication(s)'s viaroot are the same, then skip this part.
+ */
....
if (ancestor_viaroot == ancestor)
+ {
+ tables = foreach_delete_current(tables, lc2);
+ change_tables = list_append_unique_oid(change_tables,
+ relid);
+ }
Best Regards,
Takamichi Osumi
view thread (99+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Data is copied twice when specifying both child and parent table in publication
In-Reply-To: <TYCPR01MB8373BEA17F82CBFC5121AF65EDCB9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox