On Mon, Jul 19, 2021 at 2:41 PM Greg Nancarrow <gregn4...@gmail.com> wrote:
> On Fri, Jul 16, 2021 at 8:13 PM vignesh C <vignes...@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
