From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | "Bossart, Nathan" <bossartn(at)amazon(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com> |
Subject: | Re: Alter all tables in schema owner fix |
Date: | 2021-12-06 08:44:51 |
Message-ID: | CAA4eK1KmzW53rVdrdg+37qnt4+sZyXtb9hbwEr6OOAeSu--CBw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Dec 6, 2021 at 11:46 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Dec 03, 2021 at 05:20:35PM +0000, Bossart, Nathan wrote:
> > On 12/2/21, 11:57 PM, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com> wrote:
> > > Thanks for your patch.
> > > I tested it and it fixed this problem as expected. It also passed "make check-world".
> >
> > +1, the patch looks good to me, too. My only other suggestion would
> > be to move IsSchemaPublication() to pg_publication.c
>
> There is more to that, no? It seems to me that anything that opens
> PublicationNamespaceRelationId should be in pg_publication.c, so that
> would include RemovePublicationSchemaById().
>
It is currently similar to RemovePublicationById,
RemovePublicationRelById, etc. which are also in publicationcmds.c.
> If you do that,
> GetSchemaPublicationRelations() could be local to pg_publication.c.
>
> + tup = systable_getnext(scan);
> + if (HeapTupleIsValid(tup))
> + result = true;
> This can be written as just "result = HeapTupleIsValid(tup)". Anyway,
> this code also means that once we drop the schema this publication
> won't be considered anymore as a schema publication, meaning that it
> also makes this code weaker to actual cache lookup failures?
>
How, can you be a bit more specific?
> I find
> the semantics around pg_publication_namespace is bit weird because of
> that, and inconsistent with the existing
> puballtables/pg_publication_rel.
>
What do you mean by inconsistent with puballtables/pg_publication_rel?
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2021-12-06 09:05:36 | Re: pg_get_publication_tables() output duplicate relid |
Previous Message | Thomas Munro | 2021-12-06 08:17:54 | Re: stat() vs ERROR_DELETE_PENDING, round N + 1 |