| From: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Date: | 2026-05-04 12:05:48 |
| Message-ID: | CABdArM6WTm2gP4pcjVdHT1Nx6zdLKTq7nLPUkzZOhprc95a6Rw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Apr 29, 2026 at 12:02 PM Peter Smith <smithpb2250(at)gmail(dot)com> 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
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-Support-EXCEPT-clause-for-schema-level-publicatio.patch | application/octet-stream | 37.5 KB |
| v5-0002-Add-EXCEPT-support-to-ALTER-PUBLICATION-ADD-TABLE.patch | application/octet-stream | 19.8 KB |
| v5-0003-Add-EXCEPT-support-to-ALTER-PUBLICATION-SET-TABLE.patch | application/octet-stream | 20.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-05-04 12:23:04 | Re: Support logical replication of DDLs, take2 |
| Previous Message | Nisha Moond | 2026-05-04 12:05:39 | Re: Support EXCEPT for TABLES IN SCHEMA publications |