Re: Support EXCEPT for TABLES IN SCHEMA publications

From: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
Date: 2026-06-18 06:35:37
Message-ID: CABdArM48_Dm3cEcrzb0MRuaMoKhCtzbQE7mX7GyaoVOLc9fNXQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jun 17, 2026 at 7:24 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Nisha.
>
> Some review comments for v14-0001.
>
> ======
> src/test/regress/sql/publication.sql
>
> 1.
> +-- ALTER TABLE ... SET SCHEMA on a table excluded by a schema publication:
> +-- the schema-scoped exclusion is no longer meaningful once the table moves
> +-- out of its schema, so the exclution is auto-dropped.
>
> 1a.
> typo: /exclution/exclusion/
>
> ~
>
> 1b.
> Exclusions cannot be "dropped" (at least, not by a user with the DROP
> command). So even though this is correct for what is happening
> internally, maybe from the user PoV it is better to say something like
> "auto-removed" or just "removed".
>
> (Same comment may also apply to the DEBUG logging when this happened).
>
> ~~~
>
> 2.
> +-- Restore so the existing cleanup below still works
> +ALTER TABLE public.testpub_tbl_s2 SET SCHEMA pub_test;
> +DROP PUBLICATION testpub_schema_except_setsch;
>
> Why not just fix the cleanup code? It's not "existing cleanup" anyway
> -- it's part of this same patch.
>

I restored it here because patches 002 and 003 add tests that depend
on pub_test.testpub_tbl_s2.
I agree the earlier comments were confusing and made it look like it
was only for cleanup purposes. I've reworded the comments, but kept
the restore so the ALTER PUBLICATION tests can be added smoothly.

> ////////////////////
>
> Some review comments for v14-0004.
>
> ======
>
> doc/src/sgml/ref/alter_publication.sgml:
>
> 1.
> used with a publication defined with <literal>FOR TABLE</literal> or
> <literal>FOR TABLES IN SCHEMA</literal>, replaces the list of tables/schemas
> in the publication with the specified list; the existing tables or schemas
> - that were present in the publication will be removed.
> + that were present in the publication will be removed. When
> + <literal>SET TABLES IN SCHEMA</literal> is used with an
> + <literal>EXCEPT</literal> clause, the excluded tables for each schema are
> + replaced with the specified list; if <literal>EXCEPT</literal> is omitted
> + for a schema, any existing exclusions for that schema are cleared.
>
> "are cleared" or "are removed"... not sure what is better. Maybe
> "cleared" is fine.
>

okay, retained "cleared".

> ~~~
>
> 2.
> + <para>
> + The <literal>EXCEPT</literal> clause can be used with
> + <literal>ADD TABLES IN SCHEMA</literal> to exclude specific tables from a
> + schema-level publication.
> + </para>
> +
>
> This is talking about the 'ADD' variant. So really, it belongs as part
> of that 2nd para: "The first two variants modify..." because that is
> where ADD is discussed.
>

Done.

> ~~~
>
> 3.
> + <varlistentry>
> + <term><literal>EXCEPT</literal></term>
> + <listitem>
> + <para>
> + When used with <literal>ADD TABLES IN SCHEMA</literal>
> + or <literal>SET TABLES IN SCHEMA</literal>, specifies
> + tables to be excluded from the publication. Each named
> + table must belong to the schema specified in the same
> + <literal>TABLES IN SCHEMA</literal> clause. Table names may be
> + schema-qualified or unqualified; unqualified names are implicitly
> + qualified with the schema named in the same clause. See
> + <xref linkend="sql-createpublication"/> for further details on the
> + semantics of <literal>EXCEPT</literal>.
> + </para>
> + <para>
> + For <literal>FOR TABLES IN SCHEMA</literal> publications, the
> + <literal>EXCEPT</literal> clause is schema-scoped. If a table listed in
> + the <literal>EXCEPT</literal> clause is later moved to a different schema
> + using <command>ALTER TABLE ... SET SCHEMA</command>, the exclusion is
> + removed; the table will then be published if its new schema is part of a
> + publication. If the table is subsequently moved back to the original
> + schema, the exclusion is not restored, and must be re-established
> + explicitly using <command>ALTER PUBLICATION</command>. Dropping a table
> + always removes it from the <literal>EXCEPT</literal> clause,
> regardless of
> + publication type.
> + </para>
> + </listitem>
> + </varlistentry>
>
> 3a.
> TBH, I lost track of what the final decision was about keeping this EXCEPT part.
> IIUC, you wanted to defer a decision until later, but I did not
> understand the plan:
> a) Should the EXCEPT part be present for now, but consider removing it later?
> b) Should the EXCEPT part be absent for now, but consider adding it later?
>
> I prefer what you have done, but either way, we need to ensure Shlok's
> thread uses the same approach.
>

I chose (a), keeping EXCEPT part for now so others can provide
feedback and help us reach a consensus on how the documentation should
be updated.

> ~
>
> 3b.
> I think that last "Dropping a table..." sentence should be a separate
> <para>, unrelated to renaming/moving tables. Also, you don't need to
> say "regardless of publication type" -- that would be the assumption
> unless you say otherwise.
>

Fixed.
---

Attached are the updated patches v15.

--
Thanks,
Nisha

Attachment Content-Type Size
v15-0001-Support-EXCEPT-clause-for-schema-level-publicati.patch application/octet-stream 60.3 KB
v15-0002-Add-EXCEPT-support-to-ALTER-PUBLICATION-ADD-TABL.patch application/octet-stream 18.6 KB
v15-0003-Add-EXCEPT-support-to-ALTER-PUBLICATION-SET-TABL.patch application/octet-stream 23.2 KB
v15-0004-Documentation-Patch.patch application/octet-stream 10.6 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2026-06-18 06:42:39 Re: Unexpected behavior after OOM errors
Previous Message David Rowley 2026-06-18 06:27:42 Re: Fix tuple deformation with virtual generated NOT NULL columns