RE: Data is copied twice when specifying both child and parent table in publication - Mailing list pgsql-hackers

From wangw.fnst@fujitsu.com
Subject RE: Data is copied twice when specifying both child and parent table in publication
Date
Msg-id OS3PR01MB6275C00A0D4B55DDF1552AEF9E819@OS3PR01MB6275.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: Data is copied twice when specifying both child and parent table in publication  ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
List pgsql-hackers
On Mon, Mar 20, 2023 at 21:18 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
> Dear Wang,
> 
> I have tested about multilevel partitions, and it worked well.
> Followings are my comments for v18-0001.

Thanks for your comments and testing.

> 01. pg_get_publication_tables
> 
> ```
> +               ListCell   *lc;
> ```
> 
> This definition can be inside of the "for (i = 0; i < nelems; i++)".

Changed.

> 02. pg_get_publication_tables
> 
> ```
> -                        * If the publication publishes partition changes via their
> -                        * respective root partitioned tables, we must exclude partitions
> -                        * in favor of including the root partitioned tables. Otherwise,
> -                        * the function could return both the child and parent tables
> -                        * which could cause data of the child table to be
> -                        * double-published on the subscriber side.
> +                        * Publications support partitioned tables. If
> +                        * publish_via_partition_root is false, all changes are replicated
> +                        * using leaf partition identity and schema, so we only need those.
> +                        * Otherwise, get the partitioned table itself.
> ```
> 
> The comments can be  inside of the "else".

Since I think there are related operations in the function
GetAllTablesPublicationRelations, it might be better to write it above the
if-statement.

> 03. pg_get_publication_tables
> 
> ```
> +               pfree(elems);
> ```
> 
> Only elems is pfree()'d here, but how about other variable like pub_elem and
> pub_elem_tables?

Added releases to these two variables.

Regards,
Wang Wei

pgsql-hackers by date:

Previous
From: "wangw.fnst@fujitsu.com"
Date:
Subject: RE: Data is copied twice when specifying both child and parent table in publication
Next
From: Melih Mutlu
Date:
Subject: Re: Allow logical replication to copy tables in binary format