Re: bogus: logical replication rows/cols combinations

On Tue, May 24, 2022 at 3:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, May 20, 2022 at 8:36 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, May 19, 2022 at 7:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Justin Pryzby <pryzby@telsasoft.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].

[1] -
https://www.postgresql.org/message-id/OS0PR01MB5716AD7C0FE7386630BDBAAB94D79%40OS0PR01MB5716.jpnprd01.prod.outlook.com

-- 
With Regards,
Amit Kapila.