Re: Added schema level support for publication. - Mailing list pgsql-hackers

From Rahila Syed
Subject Re: Added schema level support for publication.
Date
Msg-id CAH2L28vxRLmiyZezO6DdNx-QQzGz-Ddq2=ZCYizE1y4jUkNhVA@mail.gmail.com
Whole thread Raw
In response to Re: Added schema level support for publication.  (Greg Nancarrow <gregn4422@gmail.com>)
Responses Re: Added schema level support for publication.  (vignesh C <vignesh21@gmail.com>)
List pgsql-hackers


On Mon, Jul 19, 2021 at 2:41 PM Greg Nancarrow <gregn4422@gmail.com> wrote:
On Fri, Jul 16, 2021 at 8:13 PM vignesh C <vignesh21@gmail.com> wrote:
>
> Modified.
>
> Thanks for the comments, these issues are fixed as part of the v12 patch posted at [1].
> [1]  - https://www.postgresql.org/message-id/CALDaNm3V9ny5dJM8nofLGJ3zDuDG0gS2dX%2BAhDph--U5y%2B4VbQ%40mail.gmail.com
>

There seems to be a problem with ALTER PUBLICATION ... SET TABLE ...
After that command, it still regards it as an empty (e) publication,
so I can then ALTER PUBLICATION ... ADD SCHEMA ...


One issue here is that the code to update publication type is missing 
in AlterPublicationTables for SET TABLE command.

More broadly, I am not clear about the behaviour of the patch when a
publication is created to publish only certain tables, and is later altered to publish 
a whole schema. I think such behaviour is legitimate. However,
AFAIU as per current code we can't update the publication type 
from PUBTYPE_TABLE to PUBTYPE_SCHEMA. 

I have some review comments as follows:
1. 
In ConvertSchemaSpecListToOidList(List *schemas) function: 
 +     search_path = fetch_search_path(false);
 +                               nspname = get_namespace_name(linitial_oid(search_path));
 +                               if (nspname == NULL)    /* recently-deleted namespace? */
 +                                       ereport(ERROR,
 +                                                       errcode(ERRCODE_UNDEFINED_SCHEMA),
 +                                                       errmsg("no schema has been selected"));
 +
 +                               schemoid = get_namespace_oid(nspname, false);
 +                               break;

The call get_namespace_oid() is perhaps not needed as fetch_search_path already fetches oids and simply
doing Schema oid = liinital_oid(search_path)); should be enough.

2. In the same function should there be an if else condition block instead of a switch case as
there are only two cases.


Thank you,
Rahila Syed

pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Using each rel as both outer and inner for JOIN_ANTI
Next
From: Tom Lane
Date:
Subject: Re: Have I found an interval arithmetic bug?