Re: Skipping schema changes in publication

On Fri, 26 Dec 2025 at 15:27, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Dec 23, 2025 at 12:03 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> >
> > I have addressed the remaining comments, did some cosmetic changes and
> > addressed the comment shared by Shveta in [2].
> > [1]: https://www.postgresql.org/message-id/CAA4eK1+rnjBOvkiQC2r4LuTwuje653iVPPAXcmJZXPpKvsNbOQ@mail.gmail.com
> > [2]: https://www.postgresql.org/message-id/CAJpy0uCf5tXvqyVS3GQzU9J5HdSLAxX6Lxt1UKY4HJ8qnimCAw%40mail.gmail.com
> >
>
> Thank You for the patch. Please find a few comments:
>
> 1)
> GetTopMostAncestorInPublication():
>
> + if (list_member_oid(aexceptpubids, puboid))
> + {
> + list_free(aexceptpubids);
> + continue;
> + }
>
> We need to do 'list_free(apubids)' as well here.
>
> 2)
> GetTopMostAncestorInPublication(). Currently it has:
>
> if (list_member_oid(aexceptpubids, puboid))
> ...
> if (list_member_oid(apubids, puboid))
> ...
> else
> ...schema mapping check
>
> IMO more natural order of checks will be
>
> if (list_member_oid(apubids, puboid))
> ..
> else if (list_member_oid(aexceptpubids, puboid))
> ...
> else
> ...schema mapping check
>
While analyzing this behavior, I noticed that changes to tables listed
in the EXCEPT clause were still being published. However, those
changes were not replicated, because pg_get_publication_tables()
correctly excluded those tables, and consequently pg_subscription_rel
was populated correctly on the subscriber side.
Even though replication was prevented downstream, we should not
publish changes for tables in the EXCEPT list in the first place.
Publishing them is unnecessary and inconsistent with the publication
definition. This issue has been addressed in the latest version of the
patch
And the above two changes are not required.

> 3)
> +/*
> + * Return the list of relation OIDs excluded from a publication.
> + * This is only applicable for FOR ALL TABLES publications.
> + */
> +List *
> +GetPublicationExcludedRelations(Oid pubid, PublicationPartOpt pub_partopt)
>
> a) Since now 'Relations' term means both tables and sequences, but
> here we mean only Tables, we can rename it to have 'Tables' rather
> than 'Relations'
>
> b) Similar to GetAllPublicationRelations which is for 'ALL Tables'
> pub, we can rename it to have 'All'
>
> So the name can be 'GetAllPublicationExcludedTables' to be more clear.
>
> Also we can move this function close to GetAllPublicationRelations as
> it is more related to that.
>
> 4)
> ObjectsInPublicationToOids()
> + case PUBLICATIONOBJ_EXCEPT_TABLE:
> + pubobj->pubtable->except = true;
> + *rels = lappend(*rels, pubobj->pubtable);
> + break;
>
> Let me know when this will be hit when we already have
> 'ObjectsInAllPublicationToOids' in place?
I analysed it and found that we can eliminate the use of
'ObjectsInAllPublicationToOids'. This requires a change in gram.y.
Made the changes in the latest patch.

>
> 5)
> get_rel_sync_entry():
> + level++;
> + GetRelationPublications(ancestor, NULL, &aexceptpubids);
> +
> + if (!list_member_oid(aexceptpubids, pub->oid))
> + {
> + pub_relid = ancestor;
> + ancestor_level = level;
> + }
> + }
>
> Consider the following table structure:
> t1 has a partition p1, which in turn has a child partition
> child_part1. When publish_via_partition_root is set to true, any
> changes made to child_part1 are replicated through t1. If we add t1 to
> the EXCEPT list, get_rel_sync_entry() still marks p1 as an ancestor to
> publish changes or child_part1. Is it correct?
>
This change is not required. See reply to comment 1 and 2.

> 6)
> RelationBuildPublicationDesc() also needs some more analysis about
> getting and setting ancestor part for above case.
>
The case for partition tables can be tricky. Currently I am analysing
the behaviour of partitioned tables with the EXCEPT TABLE option in
general.
Will address it in the next version upon further analysing.

> 7)
> Currently the way we deal with the except table in pg_dump.c differs
> from how we deal with included-table. To explain the same, how about
> adding below comment in getPublications() just before we fetch
> except-list:
>
> We process EXCEPT TABLES here instead of in getPublicationTables(),
> and output them directly in dumpPublication(). This differs from the
> approach used in dumpPublicationTable() and
> dumpPublicationNamespace(). Following that approach would require
> dumping table additions later as ALTER PUBLICATION … ADD EXCEPT, which
> is currently not supported.

Also addressed the remaining comments. I have also addressed the
comments by Peter in [1]. I have also done some minor cosmetic
changes.
[1]: https://www.postgresql.org/message-id/CAHut+PuZX_7Ot-oh5iqGLBRrZBS5ewDnHa91mJi2Y09uCRfixg@mail.gmail.com

Thanks,
Shlok Kyal