From: | "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com> |
Subject: | RE: Data is copied twice when specifying both child and parent table in publication |
Date: | 2022-05-13 09:41:42 |
Message-ID: | OSZPR01MB62789D224DEEBA60924301599ECA9@OSZPR01MB6278.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, May 13, 2022 1:59 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Fri, May 13, 2022 at 7:32 AM wangw(dot)fnst(at)fujitsu(dot)com
> <wangw(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Attach the patches.(Only changed the patch for HEAD.).
> >
>
> Few comments:
> =============
Thanks for your comments.
> 1.
> @@ -1135,6 +1172,15 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
> if (publication->pubviaroot)
> tables = filter_partitions(tables);
> }
> + pfree(elems);
> +
> + /*
> + * We need an additional filter for this case : A partition table is
> + * published in a publication with viaroot, and its parent or child
> + * table is published in another publication without viaroot. In this
> + * case, we should publish only parent table.
> + */
> + tables = filter_partitions(tables);
>
> Do we need to filter partitions twice? Can't we check if any of the publications
> has 'pubviaroot' option set, if so, call filter_partitions at the end?
Improve it according to your suggestion.
> 2. " FROM pg_class c JOIN pg_namespace n"
> + " ON n.oid = c.relnamespace,"
> + " LATERAL pg_get_publication_tables(array[ %s ]) gst"
>
> Here, it is better to have an alias name as gpt.
Improve it according to your suggestion.
> 3.
> }
> + pfree(elems);
> +
>
> An extra line between these two lines makes it looks slightly better.
Improve it according to your suggestion.
> 4. Not able to apply patch cleanly.
> patching file src/test/subscription/t/013_partition.pl
> Hunk #1 FAILED at 477.
> Hunk #2 FAILED at 556.
> Hunk #3 FAILED at 584.
> 3 out of 3 hunks FAILED -- saving rejects to file
> src/test/subscription/t/013_partition.pl.rej
> patching file src/test/subscription/t/028_row_filter.pl
> Hunk #1 succeeded at 394 (offset 1 line).
> Hunk #2 FAILED at 722.
> 1 out of 2 hunks FAILED -- saving rejects to file
> src/test/subscription/t/028_row_filter.pl.rej
> patching file src/test/subscription/t/031_column_list.pl
> Hunk #1 succeeded at 948 (offset -92 lines).
> Hunk #2 FAILED at 1050.
> 1 out of 2 hunks FAILED -- saving rejects to file
> src/test/subscription/t/031_column_list.pl.rej
New patch could apply patch cleanly now.
Attach the patches.(Only changed the patch for HEAD.).
1. Optimize the code. Reduce calls to function filter_partitions. [suggestions by Amit-san]
2. Improve the alias name in SQL. [suggestions by Amit-san]
3. Improve coding alignments. [suggestions by Amit-san]
4. Do some optimizations for list Concatenate.
Regards,
Wang wei
Attachment | Content-Type | Size |
---|---|---|
HEAD_v4-0001-Fix-data-replicated-twice-when-specifying-PUBLISH.patch | application/octet-stream | 13.4 KB |
REL14_v4-0001-Fix-data-replicated-twice-when-specifying-PUBLISH.patch | application/octet-stream | 5.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | shiy.fnst@fujitsu.com | 2022-05-13 09:57:15 | RE: Perform streaming logical transactions by background workers and parallel apply |
Previous Message | Etsuro Fujita | 2022-05-13 09:41:31 | Re: postgres_fdw "parallel_commit" docs |