public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
To: Amit Kapila <[email protected]>
To: Peter Smith <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Takamichi Osumi (Fujitsu) <[email protected]>
Cc: [email protected] <[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]>
Subject: RE: Data is copied twice when specifying both child and parent table in publication
Date: Wed, 22 Mar 2023 10:09:05 +0000
Message-ID: <OS3PR01MB6275350A29E2A3B9156D08089E869@OS3PR01MB6275.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TYAPR01MB586687B6DE52089302EC513FF5869@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <OS3PR01MB62751414E4DABF144F5E481C9E529@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAHut+PtTN1Udug3x1eZnDdh8Z6PL5VN=xz1b44VbB9T1z_PtcQ@mail.gmail.com>
	<OS3PR01MB6275A9B8C65C381C6828DF9D9E549@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAHut+PvaSDuZ7PhzZOm4c2u-sbN4DUsZr22qyYQ8dncUpqMpXg@mail.gmail.com>
	<OS3PR01MB6275843B2BBE92870F7881C19E299@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAHut+Pt+1PNx6VsZ-xKzAU-18HmNXhjCC1TGakKX46Wg7YNT1Q@mail.gmail.com>
	<OS3PR01MB6275FB5397C6A647F262A3A69E009@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CALDaNm3ZzNJ6Y1iX=PBYCcCU+EPm4=9LVXFin5gvQ9XkEzc8UQ@mail.gmail.com>
	<OS3PR01MB6275C62FF1D1CDE4FCBF61339E079@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS3PR01MB62758DFA4FE5BFA7CA0815329ED89@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1LCzvPLVHHXn+VcGmFHApKtbpaNF1UFed3qE8=GhPzPGw@mail.gmail.com>
	<OSZPR01MB62783EDCE9DDC21D35B789209EBD9@OSZPR01MB6278.jpnprd01.prod.outlook.com>
	<CAHut+PuNsvO9o9XzeJuSLsAsndgCKVphDPBRqYuOTy2bR28E+g@mail.gmail.com>
	<CAA4eK1Jpj2Yj5ZWwt4z-qM2fS0c3ZF3_xqAgor-Ziiru00LKHg@mail.gmail.com>
	<OS3PR01MB6275D2940C734AF8C77435739E819@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<TYAPR01MB586687B6DE52089302EC513FF5869@TYAPR01MB5866.jpnprd01.prod.outlook.com>

On Wed, Mar 22, 2023 at 14:32 PM Kuroda, Hayato/黒田 隼人 <[email protected]> wrote:
> Dear Wang,
> 
> Thank you for updating patch! Following are comments form v19-0001.

Thanks for your comments.

> 01. logical-replication.sgml
> 
> I found a following statement in logical-replication.sgml. I think this may cause
> mis-reading because it's OK when publishers list partitions and publish_via_root
> is true.
> 
> ```
>   <para>
>    A subscriber node may have multiple subscriptions if desired.  It is
>    possible to define multiple subscriptions between a single
>    publisher-subscriber pair, in which case care must be taken to ensure
>    that the subscribed publication objects don't overlap.
>   </para>
> ```
> 
> How about adding "If publications are set publish_via_partition_root as true and
> they publish partitions that have same partitioned table, only a change to
> partitioned
> table is published from the publisher."or something like that?

I think these seem to be two different scenarios: The scenario mentioned here is
multiple subscriptions at the subscription node, while the scenario we fixed
this time is a single subscription at the subscription node. So, it seems that
these two notes are not strongly related.

> 02. filter_partitions
> 
> IIUC this function can refactor like following to avoid "skip" flag.
> How do you think?
> 
> ```
> @@ -209,7 +209,6 @@ filter_partitions(List *table_infos)
> 
>         foreach(lc, table_infos)
>         {
> -               bool                            skip = false;
>                 List                       *ancestors = NIL;
>                 ListCell                   *lc2;
>                 published_rel      *table_info = (published_rel *) lfirst(lc);
> @@ -224,13 +223,10 @@ filter_partitions(List *table_infos)
>                         /* Is ancestor exists in the published table list? */
>                         if (is_ancestor_member_tableinfos(ancestor, table_infos))
>                         {
> -                               skip = true;
> +                               table_infos = foreach_delete_current(table_infos, lc);
>                                 break;
>                         }
>                 }
> -
> -               if (skip)
> -                       table_infos = foreach_delete_current(table_infos, lc);
>         }
>  }
> ```

I think this approach deletes the cell of the list of the outer loop in the
inner loop. IIUC, we can only use function foreach_delete_current in the current
loop to delete the cell of the current loop.

> 03. fetch_table_list
> 
> ```
> +       /* Get the list of tables from the publisher. */
> +       if (server_version >= 160000)
> ```
> 
> I think boolean variable can be used to check it like check_columnlist.
> How about "use_extended_function" or something?

Since we only need it once, I think it's fine not to add a new variable.

Regards,
Wang Wei


view thread (23+ 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], [email protected], [email protected], [email protected]
  Subject: RE: Data is copied twice when specifying both child and parent table in publication
  In-Reply-To: <OS3PR01MB6275350A29E2A3B9156D08089E869@OS3PR01MB6275.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