Re: Support EXCEPT for TABLES IN SCHEMA publications

On Fri, May 29, 2026 at 11:01 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Nisha.
>
> Some review comments for patch v7-0001.
>

Thanks for the review.

>
> publication_add_relation:
>
> 2.
> + if (is_except)
> + ereport(ERROR,
> + (errcode(ERRCODE_DUPLICATE_OBJECT),
> + errmsg("relation \"%s\" cannot be added because it is excluded from
> publication \"%s\"",
> + RelationGetQualifiedRelationName(targetrel),
> + pub->name)));
>
> I am unsure about the changed wording from "table" to "relation". IIUC
> we say "relation" when it could be either a table or a sequence. So
> maybe "table" is correct for your patch;l OTHOH this should change to
> "relation" by Shlok's EXCEPT SEQUENCE patch [1].
>

Okay, makes sense, changed back to "table".

> ~~~
>
> 3.
> + ereport(ERROR,
> + (errcode(ERRCODE_DUPLICATE_OBJECT),
> + errmsg("relation \"%s\" is already member of publication \"%s\"",
> + RelationGetQualifiedRelationName(targetrel), pub->name)));
>
> IMO making everything fully qualified like this would be a good
> change, but doing it here perhaps does not belong in your patch. I
> have resurrected this question in the other thread [2], which would
> affect not only this statement. but many others. Please post your
> opinion about this on that other thread.
>

That is fine with me. I've reverted the change from my patch.

> ======
> src/backend/commands/publicationcmds.c
>
> ObjectsInPublicationToOids:
>
> 4.
> static void
> ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
> - List **rels, List **exceptrels, List **schemas)
> + List **rels, List **except_rel_names, List **schemas)
>
> Is `except_rel_names` an accurate name? IMO it makes it sound like
> it's a list of char* names, but IIUC that is not the case; aren't
> these PublicationTable objects? Would something like
> `except_pubtables` be more correct?
>

Yes these are PublicationTable objects, changed the name as sugegsted.

> ~~~
>
> 9.
> BTW, the current code is not able to handle multiple schemas.
>
> So, this works:
> test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA myschema <TAB>
> EXCEPT ( TABLE WITH (
>
> but, this doesn't do anything:
> test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema <TAB>
>

I think the above preserves the existing behavior. Currently, we do
not suggest "WITH (" after the second schema onwards. To support this
properly, we would also need to handle "WITH (" suggestions for
subsequent schema entries.

I’ve created a top-up patch (patch-002) for this. I can merge it if we
want to change the current behavior. Let me know your thoughts.
~~~~

Attached is the updated v8 patch set.
Addressed all of the above comments, along with the patch v7-0002
comments from [1]. Patch-0003 has also been updated accordingly.

[1] https://www.postgresql.org/message-id/CAHut%2BPuhL7Xj8UAK0yBmbbDsCC9xvRVmreCC_yxr%2BbMfc-dt5g%40mail.gmail.com

--
Thanks,
Nisha