Re: Data is copied twice when specifying both child and parent table in publication

2022年10月17日(月) 14:49 wangw(dot)fnst(at)fujitsu(dot)com <wangw(dot)fnst(at)fujitsu(dot)com>:
>
> On Wed, Oct 5, 2022 at 11:08 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > Hi Wang-san. Here are my review comments for HEAD_v12-0001 patch.
>
> Thanks for your comments.
>
> > ======
> >
> > 1. Missing documentation.
> >
> > In [1] you wrote:
> > > I think the behaviour of multiple publications with parameter
> > publish_via_partition_root could be added to the pg-doc later in a separate
> > patch.
> >
> > ~
> >
> > That doesn't seem right to me. IMO the related documentation updates
> > cannot really be separated from this patch. Otherwise, what's the
> > alternative? Push this change, and then (while waiting for the
> > documentation patch) users will just have to use trial and error to
> > guess how it works...?
>
> I tried to add related documentation in a separate patch (HEAD_v13-0002*).
>
> > ------
> >
> > 2. src/backend/catalog/pg_publication.c
> >
> > + typedef struct
> > + {
> > + Oid relid; /* OID of published table */
> > + Oid pubid; /* OID of publication that publishes this
> > + * table. */
> > + } published_rel;
> >
> > 2a.
> > I think that should be added to typedefs.list
>
> Added.
>
> > ~
> >
> > 2b.
> > Maybe this also needs some comment to clarify that there will be
> > *multiple* of these structures in scenarios where the same table is
> > published by different publications in the array passed.
>
> Added the comments.
>
> > ------
> >
> > 3. QUESTION - pg_get_publication_tables / fetch_table_list
> >
> > When the same table is published by different publications (but there
> > are other differences like row-filters/column-lists in each
> > publication) the result tuple of this function does not include the
> > pubid. Maybe the SQL of pg_publication_tables/fetch_table_list() is OK
> > as-is but how does it manage to associate each table with the correct
> > tuple?
> >
> > I know it apparently all seems to work but I’m not how does that
> > happen? Can you explain why a puboid is not needed for the result
> > tuple of this function?
>
> Sorry, I am not sure I understand your question.
> I try to answer your question by explaining the two functions you mentioned:
>
> First, the function pg_get_publication_tables gets the list (see table_infos)
> that included published table and the corresponding publication. Then based
> on this list, the function pg_get_publication_tables returns information
> (scheme, relname, row filter and column list) about the published tables in the
> publications list. It just doesn't return pubid.
>
> Then, the SQL in the function fetch_table_list will get the columns in the
> column list from pg_attribute. (This is to return all columns when the column
> list is not specified)
>
> > ~~
> >
> > test_pub=# create table t1(a int, b int, c int);
> > CREATE TABLE
> > test_pub=# create publication pub1 for table t1(a) where (a > 99);
> > CREATE PUBLICATION
> > test_pub=# create publication pub2 for table t1(a,b) where (b < 33);
> > CREATE PUBLICATION
> >
> > Following seems OK when I swap orders of publication names...
> >
> > test_pub=# SELECT gpt.relid, gpt.attrs, pg_get_expr(gpt.qual,
> > gpt.relid) AS rowfilter from pg_get_publication_tables(VARIADIC
> > ARRAY['pub2','pub1']) gpt(relid, attrs, qual);
> > relid | attrs | rowfilter
> > -------+-------+-----------
> > 16385 | 1 2 | (b < 33)
> > 16385 | 1 | (a > 99)
> > (2 rows)
> >
> > test_pub=# SELECT gpt.relid, gpt.attrs, pg_get_expr(gpt.qual,
> > gpt.relid) AS rowfilter from pg_get_publication_tables(VARIADIC
> > ARRAY['pub1','pub2']) gpt(relid, attrs, qual);
> > relid | attrs | rowfilter
> > -------+-------+-----------
> > 16385 | 1 | (a > 99)
> > 16385 | 1 2 | (b < 33)
> > (2 rows)
> >
> > But what about this (this is similar to the SQL fragment from
> > fetch_table_list); I swapped the pub names but the results are the
> > same...
> >
> > test_pub=# SELECT pg_get_publication_tables(VARIADIC
> > array_agg(p.pubname)) from pg_publication p where pubname
> > IN('pub2','pub1');
> >
> > pg_get_publication_tables
> >
> > -------------------------------------------------------------------------------------------------
> > ---------------------------------------------------------------------
> > -------------------------------------------------------------------------------------------------
> > ---------------------------------------------------------------------
> > -------------------------------------------------------------------
> > (16385,1,"{OPEXPR :opno 521 :opfuncid 147 :opresulttype 16 :opretset
> > false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 1
> > :vartype 23 :vartypmod -1 :var
> > collid 0 :varlevelsup 0 :varnosyn 1 :varattnosyn 1 :location 47}
> > {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4
> > :constbyval true :constisnull false :
> > location 51 :constvalue 4 [ 99 0 0 0 0 0 0 0 ]}) :location 49}")
> > (16385,"1 2","{OPEXPR :opno 97 :opfuncid 66 :opresulttype 16
> > :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1
> > :varattno 2 :vartype 23 :vartypmod -1 :v
> > arcollid 0 :varlevelsup 0 :varnosyn 1 :varattnosyn 2 :location 49}
> > {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4
> > :constbyval true :constisnull false
> > :location 53 :constvalue 4 [ 33 0 0 0 0 0 0 0 ]}) :location 51}")
> > (2 rows)
> >
> > test_pub=# SELECT pg_get_publication_tables(VARIADIC
> > array_agg(p.pubname)) from pg_publication p where pubname
> > IN('pub1','pub2');
> >
> > pg_get_publication_tables
> >
> > -------------------------------------------------------------------------------------------------
> > ---------------------------------------------------------------------
> > -------------------------------------------------------------------------------------------------
> > ---------------------------------------------------------------------
> > -------------------------------------------------------------------
> > (16385,1,"{OPEXPR :opno 521 :opfuncid 147 :opresulttype 16 :opretset
> > false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 1
> > :vartype 23 :vartypmod -1 :var
> > collid 0 :varlevelsup 0 :varnosyn 1 :varattnosyn 1 :location 47}
> > {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4
> > :constbyval true :constisnull false :
> > location 51 :constvalue 4 [ 99 0 0 0 0 0 0 0 ]}) :location 49}")
> > (16385,"1 2","{OPEXPR :opno 97 :opfuncid 66 :opresulttype 16
> > :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1
> > :varattno 2 :vartype 23 :vartypmod -1 :v
> > arcollid 0 :varlevelsup 0 :varnosyn 1 :varattnosyn 2 :location 49}
> > {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4
> > :constbyval true :constisnull false
> > :location 53 :constvalue 4 [ 33 0 0 0 0 0 0 0 ]}) :location 51}")
> > (2 rows)
>
> I think this is because the usage of SELECT statement. The order seems depend
> on pg_publication. Such as:
>
> postgres=# SELECT array_agg(p.pubname) FROM pg_publication p WHERE pubname IN ('pub1','pub2');
> array_agg
> -------------
> {pub1,pub2}
> (1 row)
>
> postgres=# SELECT array_agg(p.pubname) FROM pg_publication p WHERE pubname IN ('pub2','pub1');
> array_agg
> -------------
> {pub1,pub2}
> (1 row)
>
> Attach the new patch set.

This entry was marked as "Needs review" in the CommitFest app but cfbot
reports the patch no longer applies.

We've marked it as "Waiting on Author". As CommitFest 2022-11 is
currently underway, this would be an excellent time update the patch.

Once you think the patchset is ready for review again, you (or any
interested party) can move the patch entry forward by visiting

https://commitfest.postgresql.org/40/3623/

and changing the status to "Needs review".

Thanks

Ian Barwick