On Fri, Oct 21, 2022 at 17:02 PM Peter Smith <smithpb2...@gmail.com> wrote: >
Thanks for your comments. Sorry for not replying in time. > On Mon, Oct 17, 2022 at 4:49 PM wangw.f...@fujitsu.com > <wangw.f...@fujitsu.com> wrote: > > > > On Wed, Oct 5, 2022 at 11:08 AM Peter Smith <smithpb2...@gmail.com> > wrote: > > > Hi Wang-san. Here are my review comments for HEAD_v12-0001 patch. > > > ... > > > > > > 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) > > > > I meant, for example, if the different publications specified > different col-lists for the same table then IIUC the > fetch_table_lists() is going to return 2 list elements > (schema,rel_name,row_filter,col_list). But when the schema/rel_name > are the same for 2 elements then (without also a pubid) how are you > going to know where the list element came from, and how come that is > not important? > > > > ~~ > > > > > > 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) > > > > Right, so I felt it was a bit dubious that the result of the function > “seems to depend on” something. That’s why I was asking why the list > elements did not include a pubid. Then a caller could be certain what > element belonged with what publication. It's not quite clear to me why > that is not important for this patch - but anyway, even if it's not > necessary for this patch's usage, this is a function that is exposed > to users who might have different needs/expectations than this patch > has, so shouldn't the result be less fuzzy for them? Yes, I agree that there may be such a need in the future. Added 'pubid' to the output of this function. BTW, I think the usage of the function pg_get_publication_tables is not documented in the pg-doc now, it doesn't seem to be a function provided to users. So I didn't modify the documentation. Attach new patches. Regards, Wang wei
HEAD_v14-0001-Fix-data-replicated-twice-when-specifying-publis.patch
Description: HEAD_v14-0001-Fix-data-replicated-twice-when-specifying-publis.patch
HEAD_v14-0002-Add-clarification-for-the-behaviour-of-the-publi.patch
Description: HEAD_v14-0002-Add-clarification-for-the-behaviour-of-the-publi.patch
REL15_v14-0001-Fix-data-replicated-twice-when-specifying-publis_patch
Description: REL15_v14-0001-Fix-data-replicated-twice-when-specifying-publis_patch
REL14_v14-0001-Fix-data-replicated-twice-when-specifying-publis_patch
Description: REL14_v14-0001-Fix-data-replicated-twice-when-specifying-publis_patch