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-26 03:26:32 |
Message-ID: | CAA4eK1JptALywjO4z1FShahC-bm0P0pOYhrrpkL8L_kJzbqVFg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, May 24, 2022 at 3:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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?
>
Unless someone has objections or thinks otherwise, I am planning to
proceed with the approach of extending the function/view (patch for
which is already committed) and using it to prohibit the combinations
of publications having different column lists as is done in the
currently proposed patch [1].
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2022-05-26 03:40:13 | Re: ccache, MSVC, and meson |
Previous Message | Justin Pryzby | 2022-05-26 03:26:12 | Re: ccache, MSVC, and meson |