Re: Support EXCEPT for TABLES IN SCHEMA publications

On Tue, Jun 30, 2026 at 3:47 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Mon, Jun 22, 2026 at 6:20 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > Attached updated patches v16. No changes in 0001 to 0003.
> >
>
> Thanks Nisha. I have resumed review of patch. I have not goen through
> complete patch yet, but please find a few initial comments:
>

Thanks for review Shveta.

>
> 1)
> I get this during compilation:
>
> pgoutput.c:2232:45: warning: declaration of ‘except_pubids’ shadows a
> previous local [-Wshadow=compatible-local]
>  2232 |                                 List       *except_pubids = NIL;
>       |                                             ^~~~~~~~~~~~~
> pgoutput.c:2100:29: note: shadowed declaration is here
>  2100 |                 List       *except_pubids;
>

An existing variable exceptpubids was renamed to except_pubids by
mistake while addressing comment #5 at [1].
Fixed now.

>
> 2)
> I am checking the flow:
> get_rel_sync_entry-->GetTopMostAncestorInPublication
>
> @@ -2267,7 +2290,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
>
>   ancestor = GetTopMostAncestorInPublication(pub->oid,
>      ancestors,
> -    &level);
> +    &level,
> +    except_pubids);
>
> a)
> For what all publications, flow will come to above point. I guess it
> can come for a explict table pub or a schema pub. For a schmea pub,
> except_pubids make sense but not for a explicit-table pub alone.
> Please add a comment atop GetTopMostAncestorInPublication call here to
> explain
> the scenario adn then the need of except_pubids.
>
> b)
> We have got except_pubids in the begining of the function as:
> except_pubids = GetRelationExcludedPublications(root_relid);
>
> Now inside GetTopMostAncestorInPublication(), we are keeping
> 'except_pubids' as constant while moving through all the ancestors. We
> are fetching GetRelationIncludedPublications and GetSchemaPublications
> for each ancestor but are not computing
> GetRelationExcludedPublications for each ancestor. It may not be
> obvious (for many new readers) why 'except_pubids' is not fetched
> again for each ancestor. Please add a comment there (perhaps because
> EXCEPT list can not have any partition and can have only root)
>

Added comments for both the cases.

> 3)
> The error message below and its associated validation logic are
> duplicated in three places:
>
> "cannot appear in both the table list and the EXCEPT clause"
>
> Do you think we could factor this into a small helper function and
> reuse it everywhere? That would make the code more modular and
> simplify future changes to validation or error messages.
>

Done.
~~~

Besides the above comments, I found and fixed a few additional issues
during further testing:
1) Fixed an issue where UPDATE/DELETE on an excluded table without
replica identity was still blocked. e.g.,

postgres=# create publication pub_h2 for tables in schema s1 except ( table t1);
postgres=# update s1.t1 set c1=3 where c1=1;
ERROR: cannot update table "t1" because it does not have a replica
identity and publishes updates
HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.

Excluded tables should not be treated as published for update/delete checks.

2) Fixed a bug in RemoveSchemaPubExceptForRel(), where a no-op command
such as ALTER TABLE s.t SET SCHEMA s incorrectly removed the table
from the EXCEPT list.

3) Fixed is_table_publication() logic. It relied on the first
pg_publication_rel row for a publication, which is no longer reliable
because with this feature we can have multiple rows with different
prexcept values for the same publication.
A publication can have both FOR TABLE (prexcept =false) and FOR TABLES
IN SCHEMA (prexcept=false) entries.

4) Fixed a bug in PublicationDropTables() where "ALTER PUBLICATION p
DROP TABLE t" could silently remove an EXCEPT entry. DROP TABLE should
only apply to explicitly added FOR TABLE members; EXCEPT entries are
not publication members and should not be removed through this path.
For example: Before Fix:
  create publication pub1 for tables in schema s1 except ( table t1);
  alter publication pub1 drop table s1.t1;
  ALTER PUBLICATION
-- the Except tables: entry gets deleted.

After the fix (v17):
  alter publication pub1 drop table s1.t1;
  ERROR:  relation "t1" is not part of the publication
~~~

Attached are the v17 patches.

[1] https://www.postgresql.org/message-id/CAHut%2BPuiK_Pa%3DBkSgBxYzqf1PYh%2BmcUcUQCr8r1e69-y1r%2Bhhw%40mail.gmail.com
--
Thanks,
Nisha