Re: bogus: logical replication rows/cols combinations

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Hou, Zhijie/侯 志杰 <houzj(dot)fnst(at)fujitsu(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Subject: Re: bogus: logical replication rows/cols combinations
Date: 2022-05-24 09:49:53
Message-ID: CAA4eK1Kk5ZFhHR_1V-MgXYtaED74g35iGeC90ECXdDM0jGHbSw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 20, 2022 at 8:36 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, May 19, 2022 at 7:54 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> > > On Thu, May 19, 2022 at 10:33:13AM +0530, Amit Kapila wrote:
> > >> I have committed the first patch after fixing this part. It seems Tom
> > >> is not very happy doing this after beta-1 [1]. The reason we get this
> > >> information via this view (and underlying function) is that it
> > >> simplifies the queries on the subscriber-side as you can see in the
> > >> second patch. The query change is as below:
> > >> [1] - https://www.postgresql.org/message-id/91075.1652929852%40sss.pgh.pa.us
> >
> > > I think Tom's concern is that adding information to a view seems like adding a
> > > feature that hadn't previously been contemplated.
> > > (Catalog changes themselves are not prohibited during the beta period).
> >
> > It certainly smells like a new feature, but my concern was more around the
> > post-beta catalog change. We do those only if really forced to, and the
> > explanation in the commit message didn't satisfy me as to why it was
> > necessary. This explanation isn't much better --- if we're trying to
> > prohibit a certain class of publication definitions, what good does it do
> > to check that on the subscriber side?
> >
>
> It is required on the subscriber side because prohibition is only for
> the cases where multiple publications are combined. We disallow the
> cases where the column list is different for the same table when
> combining publications. For example:
>
> Publisher-side:
> Create table tab(c1 int, c2 int);
> Create Publication pub1 for table tab(c1);
> Create Publication pub1 for table tab(c2);
>
> Subscriber-side:
> Create Subscription sub1 Connection 'dbname=postgres' Publication pub1, pub2;
>
> We want to prohibit such cases. So, it would be better to check at the
> time of 'Create Subscription' to validate such combinations and
> prohibit them. To achieve that we extended the existing function
> pg_get_publication_tables() and view pg_publication_tables to expose
> the column list and verify such a combination. We primarily need
> column list information for this prohibition but it appeared natural
> to expose the row filter.
>

I still feel that the current approach to extend the underlying
function and view is a better idea but if you and or others are not
convinced then we can try to achieve it by extending the existing
query on the subscriber side as mentioned in my previous email [1].
Kindly let me know your opinion?

[1] - https://www.postgresql.org/message-id/CAA4eK1KfL%3Dez5fKPB-0Nrgf7wiqN9bXP-YHHj2YH5utXAmjYug%40mail.gmail.com

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shinya Kato 2022-05-24 11:07:31 Re: Add --{no-,}bypassrls flags to createuser
Previous Message Nikolay Shaplov 2022-05-24 08:09:45 Re: Zstandard support for toast compression