RE: bogus: logical replication rows/cols combinations

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
Cc: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: bogus: logical replication rows/cols combinations
Date: 2022-05-17 09:10:00
Message-ID: OS0PR01MB571647F2739F710E3D1B045D94CE9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tuesday, May 17, 2022 2:53 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> Few minor comments:
> ==================
> 1.
> + <para>
> + Names of table columns included in the publication. This contains all
> + the columns of table when user didn't specify column list for the
> + table.
> + </para></entry>
>
> Can we slightly change it to: "Names of table columns included in the
> publication. This contains all the columns of the table when the user
> didn't specify the column list for the table."
>
> 2. Below comments needs to be removed from tablesync.c as we don't
> combine column lists after this patch.
>
> * For initial synchronization, column lists can be ignored in following
> * cases:
> *
> * 1) one of the subscribed publications for the table hasn't specified
> * any column list
> *
> * 2) one of the subscribed publications has puballtables set to true
> *
> * 3) one of the subscribed publications is declared as ALL TABLES IN
> * SCHEMA that includes this relation
>
> 3.
> Note that we don't support the case where column list is different for
> + * the same table when combining publications. But we still need to check
> + * all the given publication-table mappings and report an error if any
> + * publications have different column list.
>
> Can we change this comment to: "Note that we don't support the case
> where the column list is different for the same table when combining
> publications. But one can later change the publication so we still
> need to check all the given publication-table mappings and report an
> error if any publications have a different column list."?
>
> 4. Can we add a test for different column lists if it is not already there?

Thanks for the comments.

Attach the new version patch which addressed all the above comments and
comments from Shi yu[1] and Osumi-san[2].

[1] https://www.postgresql.org/message-id/OSZPR01MB6310F32344884F9C12F45071FDCE9%40OSZPR01MB6310.jpnprd01.prod.outlook.com
[2] https://www.postgresql.org/message-id/TYCPR01MB83736AEC2493FCBB75CC7556EDCE9%40TYCPR01MB8373.jpnprd01.prod.outlook.com

Best regards,
Hou zj

Attachment Content-Type Size
v3-0001-Extend-pg_publication_tables-to-display-column-list-.patch application/octet-stream 15.0 KB
v3-0002-Prohibit-combining-publications-with-different-colum.patch application/octet-stream 22.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2022-05-17 11:41:03 Expand palloc/pg_malloc API
Previous Message Kyotaro Horiguchi 2022-05-17 08:43:42 create_help.pl treats <literal> as replaceable