On Wed, Apr 29, 2026 at 12:02 PM Peter Smith <[email protected]> wrote: > > Hi Nisha. > > A couple of ad-hoc review comments for v4-0001... > > ====== > src/bin/pg_dump/pg_dump.c > > dumpPublicationNamespace: > > 1. > + if (!first_except) > + appendPQExpBufferStr(query, ")"); > > The logic seems ok, but the above seems a bit strange, checking > 'first_except'. Maybe renaming to 'has_except' (and some small > refactoring) would read better? > > ~~~
Thanks for pointing that out. This also needed updating after the syntax change to EXCEPT (TABLE ...). I’ve used the name "has_except", defaulting it to false for better readability. > > 2. > IIUC, the 'ALTER PUBLICATION' EXCEPT clause syntax change is not > introduced until your patch 0003. So, how does this dump code even > work when it relies on that ALTER PUBLICATION? > > Furthermore, the patch 0003 commit message says 'ALTER PUBLICATION pub > SET TABLES', but this dump code is using 'ALTER PUBLICATION pub ADD > TABLES' (note ADD v SET). Something seems suspicious... > I think there was a slight mix-up in reading the patch messages. The ALTER PUBLICATION ... EXCEPT syntax is introduced in patch-002 for ADD, not patch-003. That said, you’re right. The pg_dump part doesn’t work with patch-001 alone. I missed moving it to patch-002 while splitting the patches. Thanks for catching that; it’s now fixed. Also, patch-003 is for SET (not ADD) and includes handling for cleanup of EXCEPT entries when a schema is dropped. I’ve updated the patch message for clarity. Attached v5 patches with these updates. -- Thanks, Nisha
v5-0001-Support-EXCEPT-clause-for-schema-level-publicatio.patch
Description: Binary data
v5-0002-Add-EXCEPT-support-to-ALTER-PUBLICATION-ADD-TABLE.patch
Description: Binary data
v5-0003-Add-EXCEPT-support-to-ALTER-PUBLICATION-SET-TABLE.patch
Description: Binary data
