Thread
-
Re: Support EXCEPT for TABLES IN SCHEMA publications
Peter Smith <smithpb2250@gmail.com> — 2026-05-29T05:31:01Z
Hi Nisha. Some review comments for patch v7-0001. ====== src/backend/catalog/pg_publication.c GetTopMostAncestorInPublication: 1. - else + else if (!list_member_oid(except_pubids, puboid)) { aschemaPubids = GetSchemaPublications(get_rel_namespace(ancestor)); IIUC this `except_pubids` and `puboid` are not changing, so you do not need to be doing this member check every loop iteration. Is it better to assign some variable up-front? e.g. bool check_schemas = !list_member_oid(except_pubids, puboid); ~~~ 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]. ~~~ 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. ====== 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? ~~~ CreatePublication: 5. - List *exceptrelations = NIL; + List *except_rel_names = NIL; Same doubts about this `except_rel_names` variable name. ~~~ AlterPublication: 6. List *relations = NIL; - List *exceptrelations = NIL; + List *except_rel_names = NIL; Same doubts about this `except_rel_names` variable name. ====== src/backend/replication/pgoutput/pgoutput.c get_rel_sync_entry: 7. + if (am_partition) + { + List *part_ancestors = get_partition_ancestors(relid); + + root_relid = llast_oid(part_ancestors); + list_free(part_ancestors); I think just call this `ancestors` (not `part_ancestors`) for consistency with other code in the same function. ====== src/bin/psql/tab-complete.in.c On Thu, May 28, 2026 at 9:27 PM Nisha Moond <nisha.moond412@gmail.com> wrote: > > On Fri, May 22, 2026 at 7:57 AM Peter Smith <smithpb2250@gmail.com> wrote: ... > > 6. > > + else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES", > > "IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) && > > ends_with(prev_wd, ',')) > > + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables); > > > > I'm not sure if this is working as intended. > > > > When testing for multiple except tables I get results like: > > ---- > > test_pub=# create publication pub1 for tables IN SCHEMA myschema <TAB> > > EXCEPT ( TABLE WITH ( > > test_pub=# create publication pub1 for tables IN SCHEMA myschema > > except ( table <TAB> > > test_pub=# create publication pub1 for tables IN SCHEMA myschema > > except ( table myschema.t<TAB> > > myschema.t1 myschema.t2 myschema.t3 > > test_pub=# create publication pub1 for tables IN SCHEMA myschema > > except ( table myschema.t1,<TAB> > > information_schema. myschema. public. t1 > > t2 t3 > > ---- > > > > Note: it is offering suggstions for schema names outside of the > > "myschema". Should this code be calling > > Query_for_list_of_tables_in_schema instead of > > Query_for_list_of_tables? > > > > For this case, I don't think it's really possible to keep suggesting > after a comma. Even if we handle a fixed number of comma-separated > entries, the same problem reappears with the next entry. > Query_for_list_of_tables_in_schema needs a correct schema reference, > but with each comma the relative offset changes, so there is no single > fixed prev*_wd that can reliably point to the schema across all > entries. > > But I see, the current call to Query_for_list_of_tables is clearly > incorrect here. I have now suppressed suggestions after a comma, > instead of showing incorrect suggestions. > Thoughts? > 8. REPLY: Yeah, I don't have any good ideas how to fix this, or if a fix is even possible, but I agree that doing nothing is better than doing the wrong thing. ~~~ 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> ====== [1] Shlok EXCEPT - https://www.postgresql.org/message-id/flat/CAHut%2BPsUrYmbZ996ZybjMWvpW_ufXB8WM94pdvAPyzQpoe%2BHRA%40mail.gmail.com#579d9b99c4f620602085cc59ff0e2b7d [2] schema-qualified messages - https://www.postgresql.org/message-id/CAHut%2BPvWoOyLKFb627Ch%2BXg3TYHuHdaOZ_XmxYgKVYdOzpqFsw%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia