Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications

From: Nisha Moond <[email-masked]>
Date: Tue, 19 May 2026 16:44:09 +0530

On Fri, May 15, 2026 at 12:49 PM Peter Smith <[email-masked]> wrote:
>
> Hi Nisha.
>
> Some review comments for patch v5-0001.
>

Thanks Peter, for the review.

> ======
> src/backend/catalog/pg_publication.c
>
> publication_add_relation:
>
> 2.
> + HeapTuple existing;
>
> Not sure if this is the best name. How about "tup"?
>

Noticed that a "HeapTuple tup;" is already declared, so we can use the
existing one.

> ~~~
>
> 3.
> + bool is_except = existing_form->prexcept;
>
> This variable is used only once. Not sure if that vindicates having it.
>

the is_except value is being used after releasing the tuple and
closing the table since the next step errors out. But I have now
removed existing_form and directly extracted the value instead.

> ~~~
>
> 8.
> + list_free(schemaRels);
> + }
> + else
> + result = list_concat(result, schemaRels);
>
> Why is 'schemaRels' only being freed when there is an EXCEPT?
>

IIUC, In the EXCEPT case, relid is an Oid, so lfirst_oid() copies the
integer value from the cell, and lappend_oid() stores that value into
a new cell in result. That means result does not reference schemaRels
cells after the loop, so list_free(schemaRels) is safe.

In the else branch, list_concat() directly transfers schemaRels cells
into result. So freeing schemaRels there would corrupt the result.

> ======
> src/backend/parser/gram.y
>
> 14.
> - | ColId opt_column_list OptWhereClause
> + | ColId opt_column_list OptWhereClause opt_pub_except_clause
>   {
>   $$ = makeNode(PublicationObjSpec);
>   $$->pubobjtype = PUBLICATIONOBJ_CONTINUATION;
> + $$->except_tables = $4;
>
> This seems suspicious. You cannot have an EXCEPT clause when there is
> a column list or a WHERE clause, so what is this scenario? Maybe the
> "$$->except_tables = $4;" needs to be moved to the 'else' block?
>

This handles cases where multi-schema continuation is used along with
an EXCEPT clause, e.g.:
  create publication pub1 for tables in schema public, s1 except (table t1);
-- without the above, the EXCEPT clause would be silently ignored.

Now, I agree that the above case can also be handled by moving
"$$->except_tables = $4;" into the else branch. But then EXCEPT would
again get silently ignored for table continuations with a column-list
or where clause, e.g.,:
postgres=# create publication pub2 for table t1, t2 (c1) except (table t1);
CREATE PUBLICATION

The idea here is to collect the EXCEPT list whenever it is specified
for such continuation cases, and then explicitly raise an error in
preprocess_pubobj_list() if a table publication object contains an
EXCEPT list.

> ~~~
>
> 16.
> + if (pubobj->pubobjtype == PUBLICATIONOBJ_TABLES_IN_SCHEMA)
> + {
> + if (eobj->pubtable->relation->schemaname == NULL)
> + eobj->pubtable->relation->schemaname = pubobj->name;
> + else if (strcmp(eobj->pubtable->relation->schemaname,
> + pubobj->name) != 0)
> + ereport(ERROR,
> + errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("table \"%s.%s\" in EXCEPT clause does not belong to schema \"%s\"",
> +    eobj->pubtable->relation->schemaname,
> +    eobj->pubtable->relation->relname,
> +    pubobj->name),
> + parser_errposition(eobj->location));
>
> 16a.
> Introducing some more variables (like 'eobj_schemaname' and
> 'eobj_relname') would simplify this code quite a bit.
>

Done.

> ~
>
> 16b.
> Should make use of the recently committed function that gets
> fully-qualified rel names so you can use "%s" instead of "%s.%s".
>

We cannot use RelationGetQualifiedRelationName() in the grammar, so I
used quote_qualified_identifier() instead.
~~~~

Addressed all other comments as suggested. Please find the updated v6
patches attached.

Patch-0001: updated as per the above comments.
Patch-0002 and Patch-0003: adjusted for the Patch-0001 changes and
some code simplification in tab-complete part.

--
Thanks,
Nisha

---

From: Peter Smith <[email-masked]>
Date: Fri, 22 May 2026 12:26:35 +1000

Hi Nisha.

Here are some review comments for patch v6-0001.

======

GetTopMostAncestorInPublication:

1.
+GetTopMostAncestorInPublication(Oid puboid, List *ancestors,
+ int *ancestor_level, List *except_pubids)

I am having dificulty understanding this function. There needs to be a
description what does the input parameter 'except_pubids' mean. The
param name doesn't tell me anything much -- just that it is a list of
pubids that "something" (what?) is excluded from. And how does that
relate to the 'ancestors'?

~~~

2.
  {
  aschemaPubids = GetSchemaPublications(get_rel_namespace(ancestor));
- if (list_member_oid(aschemaPubids, puboid))
+ if (list_member_oid(aschemaPubids, puboid) &&
+ !list_member_oid(except_pubids, puboid))

Is this new code in the right place? I'm not 100% sure of the
'except_pubids' details, but shouldn't it be checked sooner? e.g.  if
we know already that this pubid is no good
(!list_member_oid(except_pubids, puboid)) then what is the point to
even assign/check aschemaPubids?

~~~

3.
+ if (is_except)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("table \"%s\" cannot be added because it is excluded from
publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel),
+ pub->name)));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("relation \"%s\" is already member of publication \"%s\"",
+ RelationGetRelationName(targetrel), pub->name)));

Fully qualified 'targetrel' in the first error, but not in the second? Is it OK?

~~~

GetAllSchemaPublicationRelations:

4.
+ List    *exceptlist = NIL;

The varname is a bit vague; it is a list of "what"? Maybe say
'except_relids' or similar.

======
src/backend/replication/pgoutput/pgoutput.c

get_rel_sync_entry:

5.
+ /*
+ * For the schema EXCEPT check, we must look up the top-most ancestor
+ * rather than the relation itself.  check_publication_add_relation()
+ * prevents individual partitions from appearing in the EXCEPT clause,
+ * so only a root (non-partition) table can have prexcept = true.
+ * Using the partition's own OID would always return NIL and miss the
+ * exclusion.
+ */
+ Oid root_relid = am_partition ?
+ llast_oid(get_partition_ancestors(relid)) : relid;
+
+ schemaExceptPubids = GetRelationExcludedPublications(root_relid);

5a.
The varname 'schemaExceptPubids' seems ambiguous. It sounds like it is
pubids that have EXCEPT SCHEMA. In the future the ALL TABLES may
introduce "EXCEPT SCHEMA", but currently there is no such thing.
Meanwhile, here I think it means "EXCEPT TABLE", so IMO that varname
needs to indicate the meaning better.

~

5b.
Actually, this is becoming a GENERAL comment. There too many ways that
these EXCEPT tables are getting named, and it is causing confusion:
- except_pubids
- exceptlist
- exceptrelations
- exceptrels
- except_relid
- except_tables
- schemaExceptPubids

Can we standardize on some common names, to make all the code more consistent?

~

5c.
Previously, the result of 'get_partition_ancestors' was being freed,
but now it is not. I'm not sure how importatnt that is, because I
found other examples in PG source code also not freeing...

======
src/bin/psql/tab-complete.in.c

6.
+ else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES",
"IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) &&
ends_with(prev_wd, ','))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);

I'm not sure if this is working as intended.

When testing for multiple except tables I get results like:
----
test_pub=# create publication pub1 for tables IN SCHEMA myschema <TAB>
EXCEPT ( TABLE  WITH (
test_pub=# create publication pub1 for tables IN SCHEMA myschema
except ( table <TAB>
test_pub=# create publication pub1 for tables IN SCHEMA myschema
except ( table myschema.t<TAB>
myschema.t1  myschema.t2  myschema.t3
test_pub=# create publication pub1 for tables IN SCHEMA myschema
except ( table myschema.t1,<TAB>
information_schema.  myschema.            public.              t1
             t2                   t3
----

Note: it is offering suggstions for schema names outside of the
"myschema". Should this code be calling
Query_for_list_of_tables_in_schema instead of
Query_for_list_of_tables?

======
src/test/regress/sql/publication.sql

7.
---- Tests for publications with SEQUENCES
+---------------------------------------------
+-- EXCEPT tests for TABLES IN SCHEMA
+---------------------------------------------
+SET client_min_messages = 'ERROR';

It looks like a previous comment for the SEQUENCES tests has been
accidentally removed.

I should be put back, and made more prominent like the other big comments.
e.g.
---------------------------------------------
-- Tests for publications with SEQUENCES
---------------------------------------------

~~~

8.
+RESET client_min_messages;
+DROP TABLE pub_test.testpub_tbl_s1, pub_test.testpub_tbl_s2;
+DROP TABLE pub_test.testpub_parted_s CASCADE;
+DROP TABLE testpub_nopk, testpub_tbl_s1;
+DROP PUBLICATION testpub_schema_except1, testpub_schema_except2,
testpub_schema_except_multi;
+

Add a "Cleanup" comment.

======
Kind Regards,
Peter Smith.
Fujitsu Australia.

---

From: Peter Smith <[email-masked]>
Date: Fri, 22 May 2026 15:30:12 +1000

Hi Nisha.

Here are some review comments for patch v6-0002.

======
Commit message

1.
Extend the EXCEPT clause support to allow tables to be excluded when
adding a schema to a publication via ALTER PUBLICATION ... ADD:

~

/ADD:/ADD./

======
doc/src/sgml/ref/alter_publication.sgml

Synopsis.

2.
-    TABLES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ...
]
+    TABLES IN SCHEMA { <replaceable
class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [
EXCEPT ( <replaceable
class="parameter">except_table_object</replaceable> [, ... ] ) ] [,
... ]

~

Probably needs to change to introduce the 'tables_in_schema' part,
same as in the CREATE PUBLICATION synopsis.

~~~

3.
+
+<phrase>and <replaceable
class="parameter">except_table_object</replaceable> is:</phrase>
+
+   [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]

Something is wrong. Now the synopsis has 'except_table_object' 2x.

~~~

4.
+  <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. <literal>EXCEPT</literal> is not supported with
+   <literal>DROP TABLES IN SCHEMA</literal>; instead, dropping a schema from
+   the publication automatically removes all of its associated
+   <literal>EXCEPT</literal> entries.
+  </para>

4a.
I didn't think you need to say it is a "schema-level" publication.
That much is obvious because it already says "TABLES IN SCHEMA".

~

4b.
Maybe do not say "is not supported", because IMO that implies it will
cause an error.

SUGGESTION (or something like this)
Using <literal>DROP TABLES IN SCHEMA</literal> on a publication will
automatically also remove any associated <literal>EXCEPT</literal>
entries.

~~~

EXCEPT

5.
+   <varlistentry>
+    <term><literal>EXCEPT ( <replaceable
class="parameter">except_table_object</replaceable> [, ... ]
)</literal></term>
+    <listitem>
+     <para>
+      Specifies tables to be excluded from a schema-level publication entry.
+      This clause may be used with <literal>ADD TABLES IN SCHEMA</literal>
+      and not with <literal>DROP TABLES IN SCHEMA</literal>.  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>
+    </listitem>
+   </varlistentry>
+

5a.
Oh! If this EXCEPT part was previously missing even for the "FOR ALL
TABLES", then IMO that is a separate bug that should be in another
thread and patched/fixed asap, then your patch should just make small
changes to to it.

~

5b.
I don't think you need to say "schema-level" here... Maybe reword like
"When used with ADD TABLES IN SCHEMA...". Anyway, all this wording
will need to change a bit after the aforementioned fix for "FOR ALL
TABLES EXCEPT" patched/pushed.

~~~

Examples

6.
+<programlisting>
+ALTER PUBLICATION sales_publication ADD TABLES IN SCHEMA sales EXCEPT
(TABLE sales.internal, sales.drafts);
+</programlisting>

It is OK left in the description, but IMO it is better if you don't
use the schema-qualified name in the actual code fragment.

======
src/backend/commands/publicationcmds.c

AlterPublicationSchemas:

7.
+ /*
+ * Increment the command counter so that is_schema_publication() in
+ * GetExcludedPublicationTables() can see the just-inserted schema
+ * rows when AlterPublicationExceptTables runs next.
+ */
+ CommandCounterIncrement();

Should this cut/paste common code be done afterwards just if
stmt->action == AP_AddObjects/SetObjects ?

~~~

AlterPublicationExceptTables:

8.
+ /*
+ * This function handles EXCEPT entries for schema-level publications
+ * only.  For FOR ALL TABLES publications, EXCEPT entries are already
+ * processed by AlterPublicationTables().
+ */
+ if (schemaidlist == NIL && !is_schema_publication(pubid))
+ return;

Huh? It seems contrary to the function comment that was also talking
about "FOR ALL TABLES".

Should this function really be called something different like
'AlterPublicationSchemaExceptTables'?

~~~

9.
+ /*
+ * EXCEPT with SET is not supported: SET replaces the schema list but does
+ * not have a well-defined semantics for merging or replacing existing
+ * except entries.  Users should DROP and re-ADD the schema with the
+ * desired EXCEPT list instead.
+ */
+ if (stmt->action == AP_SetObjects)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("EXCEPT clause is not supported with SET in ALTER PUBLICATION")));

9a.
Not sure about this comment saying "does not have a well-defined
semantics". Should you instead just have XXX comment to simply say
"Not yet implemented", because this is getting replaced later by your
patch 0003 I think.

SUGGESTION
XXX EXCEPT with SET is not currently implemented. Workaround: Users
should DROP and re-ADD the schema with the desired EXCEPT list.

~

9b.
The ereport should be temporarily (until patch 0003 is pushed) have
using an errhint to say the workaround.

~~~

10.
+ if (stmt->action == AP_AddObjects)
+ {
+ List    *rels;
+ List    *explicitrelids;
+ ListCell   *lc;
+
+ rels = OpenTableList(exceptrelations);
+
+ explicitrelids = GetIncludedPublicationRelations(pubid,
+ PUBLICATION_PART_ROOT);
+
+ /*
+ * Validate that each excluded table is not also in the explicit table
+ * list (which would be contradictory).
+ */
+ foreach(lc, rels)

Can tidy this using a foreach_ptr look instead of 'lc'.

~~~

11.
+ if (list_member_oid(explicitrelids, relid))
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("table \"%s.%s\" cannot appear in both the table list and the
EXCEPT clause",
+    get_namespace_name(relns),
+    RelationGetRelationName(pri->relation)));

Make use of the new function to get fully qualified names and replace
\"%s.%s\" with \"%s\".

~~~

12.
+ /*
+ * For FOR ALL TABLES, EXCEPT entries are processed by
+ * AlterPublicationTables(), so merge them in.  For TABLES IN SCHEMA,
+ * they are handled separately by AlterPublicationExceptTables().
+ */
+ if (stmt->for_all_tables)
+ relations = list_concat(relations, exceptrelations);
  AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext,
     schemaidlist != NIL);
  AlterPublicationSchemas(stmt, tup, schemaidlist);
+ AlterPublicationExceptTables(stmt, tup, exceptrelations, schemaidlist);

Would it be simpler if AlterPublicationExceptTables was merged (or
delegated from) the AlterPublicationSchemas?

======
src/bin/pg_dump/t/002_pg_dump.pl

13.
+ 'CREATE PUBLICATION pub11' => {
+ create_order => 50,
+ create_sql =>
+   'CREATE PUBLICATION pub11 FOR TABLES IN SCHEMA dump_test EXCEPT
(TABLE dump_test.test_table);',
+ regexp => qr/^
+ \QCREATE PUBLICATION pub11 WITH (publish = 'insert, update, delete,
truncate');\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ },
+
+ 'ALTER PUBLICATION pub11 ADD TABLES IN SCHEMA dump_test EXCEPT
(dump_test.test_table)'
+   => {
+ regexp => qr/^
+ \QALTER PUBLICATION pub11 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE ONLY test_table);\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+   },
+
+ 'CREATE PUBLICATION pub12' => {
+ create_order => 50,
+ create_sql =>
+   'CREATE PUBLICATION pub12 FOR TABLES IN SCHEMA dump_test EXCEPT
(TABLE dump_test.test_table, dump_test.test_second_table);',
+ regexp => qr/^
+ \QCREATE PUBLICATION pub12 WITH (publish = 'insert, update, delete,
truncate');\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ },
+
+ 'ALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(dump_test.test_table, dump_test.test_second_table)'
+   => {
+ regexp => qr/^
+ \QALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE ONLY test_table, TABLE ONLY test_second_table);\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+   },
+

Should not need to specify schema-qualified names in the CREATE
PUBLICATION or the ALTER PUBLICATION. I think a better test would have
one of each (e.g. don't qualify the 'dump_test.test_table') in any of
those SQL.

======

14.
+ else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD", "TABLES",
"IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) &&
ends_with(prev_wd, ','))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
+ else if (Matches("ALTER", "PUBLICATION", MatchAny, "ADD", "TABLES",
"IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) &&
!ends_with(prev_wd, ','))
+ COMPLETE_WITH(")");

I've not tested this, but the code looks the same. so I suspect this
suffers the same problem where it lists potentially all tables instead
of just the table of the current schema. Maybe this is an unavoidable
quirk...

======

15.
Should you add a some more test cases?

e.g. Pass with EXCEPT without the schema-qualified name.
e.g. Pass with multiple excepted tables.
e.g. Fail because non-existing table name.
e.g. Fail because table not in schema.
e.g. Fail syntax because missing keyword TABLE.
e.g. do a DROP TABLES IN SCHEMA to test that the except tables gove removed too

======
Kind Regards,
Peter Smith.
Fujitsu Australia.

---

From: Peter Smith <[email-masked]>
Date: Tue, 26 May 2026 15:56:45 +1000

Hi Nisha.

Some review comments for patch v6-0003.

======
Commit Message

1.
Extend AlterPublicationExceptTables() with the AP_SetObjects case,
which redefine the publication and replaces the entire EXCEPT list.

~

/redefine/redefines/

======

2.
-
-<phrase>and <replaceable
class="parameter">except_table_object</replaceable> is:</phrase>
-
-   [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]

IIUC this is just removing some duplicate entry in the synopsis that
was not supposed to be there in the first place.

~~~

3.
    The <literal>EXCEPT</literal> clause can be used with
-   <literal>ADD TABLES IN SCHEMA</literal> to exclude specific tables from a
+   <literal>ADD TABLES IN SCHEMA</literal> and
+   <literal>SET TABLES IN SCHEMA</literal> to exclude specific tables from a
    schema-level publication. <literal>EXCEPT</literal> is not supported with
    <literal>DROP TABLES IN SCHEMA</literal>; instead, dropping a schema from
    the publication automatically removes all of its associated

3a.
This whole description section seems arranged in a confusing way IMO.
Anyway, it is not all the fault of the current patch. But I don't
think it should be talking about "SET TABLES IN SCHEMA" here because
that was all mentioned already in the earlier "third variant"
paragraph.

~

3b.
That last sentence all about EXCEPT with DROP TABLES IN SCHEMA seems
redundant to me. It is not allowed by the synopsis, so there is no
possible confusion about it being supported. Isn't it better to just
say nothing?

~~~

EXCEPT

4.
      <para>
       Specifies tables to be excluded from a schema-level publication entry.
       This clause may be used with <literal>ADD TABLES IN SCHEMA</literal>
-      and not with <literal>DROP TABLES IN SCHEMA</literal>.  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
+      and <literal>SET TABLES IN SCHEMA</literal>, and not with
+      <literal>DROP TABLES IN SCHEMA</literal>.  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>

4a.
IMO there is no reason to mention the "DROP TABLES IN SCHEMA" has no
EXCEPT. That is not possible just by looking at the synopsis, so there
is no ambiguity. Why say anything at all?

~

4b.
This description about EXCEPT is missing talking about FOR ALL TABLES
EXCEPT, but IIRC I already reported that in a previous review.

~~~

EXAMPLES

5.
+   Replace the schema list of <structname>sales_publication</structname> with
+   <structname>sales</structname>, excluding only
+   <structname>sales.drafts</structname> (any previously excluded tables for
+   that schema are replaced, and schemas previously in the publication are
+   removed):

BEFORE
(any previously excluded tables for that schema are replaced, and
schemas previously in the publication are removed):

SUGGESTION
Other than sales.drafts, any previously excluded tables for schema
sales are no longer excluded. Any schemas previously in the
sales_publication are removed.

~~~

6.
+<programlisting>
+ALTER PUBLICATION sales_publication SET TABLES IN SCHEMA sales EXCEPT
(TABLE sales.drafts);
+</programlisting>

Don't fully qualify that table in the SQL example.

======

7.
- * Nothing to do if no EXCEPT entries.
+ * Nothing to do if no EXCEPT entries, except in SET: for that it is quite
+ * possible that the user has removed all exceptions, in which case we
+ * need to drop any existing ones.

Maybe reword this because it is a bit odd to have the word "except"
with the keyword "EXCEPT".

SUGGESTION
Nothing to do if there are no EXCEPT entries, unless handling the SET
command, because if the user has removed all exceptions we need to
drop any existing ones.

~~~

8.
+ {
+ List    *oldexceptrelids = NIL;
+ List    *newexceptrelids = NIL;
+ List    *delrelids = NIL;
+ List    *rels;
+ List    *explicitrelids;
+ ListCell   *lc;
+
+ rels = OpenTableList(exceptrelations);
+
+ /* Collect OIDs of the desired new except list. */
+ foreach(lc, rels)

There are multiple foreach() loops which can probably be simplified
all by using foreach_ptr(...) instead.

~~~

9.
+ /* Collect OIDs of the desired new except list. */
+ foreach(lc, rels)

/except/EXCEPT/

~~~

10.
+ ereport(ERROR,

Use the new function to get a fully qualified name and just substitute
\"%s\" instead of \"%s.%s\".

~~~

11.
+ /*
+ * Get the current set of except entries.  Only FOR ALL TABLES and
+ * schema-level publications can have except entries; for any other
+ * publication type oldexceptrelids stays NIL.
+ *
+ * Note: we check is_schema_publication() against the current catalog
+ * state (before AlterPublicationSchemas has run), so if the caller is
+ * doing SET TABLE t1 to convert a schema publication into a plain
+ * table publication, is_schema_publication() still returns true here.
+ * That is intentional: it lets us discover and clean up any stale
+ * except entries that belong to the old schema definition.
+ */
+ if (GetPublication(pubid)->alltables || is_schema_publication(pubid))
+ oldexceptrelids = GetExcludedPublicationTables(pubid,
+    PUBLICATION_PART_ROOT);
+
+ /* Build a list of old except entries not present in the new list. */
+ foreach(lc, oldexceptrelids)
+ {
+ Oid oldrelid = lfirst_oid(lc);
+
+ if (!list_member_oid(newexceptrelids, oldrelid))
+ delrelids = lappend_oid(delrelids, oldrelid);
+ }
+
+ /* Drop old except entries not present in the new list. */
+ foreach(lc, delrelids)

There are multiple comments here mentioning "except entries", but I
think they should say "EXCEPT entries".

~~~

PublicationDropSchemas:

12.
+ /*
+ * Collect prexcept rows for tables belonging to this schema before
+ * removing the schema entry.  GetExcludedPublicationTables relies on
+ * is_schema_publication(), which scans pg_publication_namespace; if
+ * this is the last schema in the publication, performDeletion() below
+ * would remove that row and make is_schema_publication() return
+ * false, tripping the assertion.
+ */

What assertion?

~~~

13.
+ foreach(elc, exceptoids)
+ {
+ Oid relid = lfirst_oid(elc);
+ Oid proid;

Maybe this can be changed to be a foreach_oid(...) loop.

======

14.
+ else if (Matches("ALTER", "PUBLICATION", MatchAny, "SET", "TABLES",
"IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) &&
ends_with(prev_wd, ','))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);

Not sure if this also ought to be
'Query_for_list_of_tables_in_schema', instead of
'Query_for_list_of_tables'.

======

15.
+-- error: EXCEPT is not allowed with DROP

I think we should keep all the SET tests together. The DROP test seems
to be between other SET tests.

~~~

16.
Perhaps there should be some more tests -- eg. a test case to hit this
new error "table \"%s.%s\" cannot appear in both the table list and
the EXCEPT clause"

======
src/test/subscription/t/037_except.pl

17.
+$node_publisher->safe_psql(
+ 'postgres', qq(
+ INSERT INTO sch1.tab_published VALUES (7);
+ INSERT INTO sch1.tab_excluded VALUES (7);
+));
+$node_publisher->wait_for_catchup('sch_sub');
+
+$result =
+  $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM sch1.tab_excluded");
+is($result, qq(7),
+ 'ALTER ... SET TABLES IN SCHEMA EXCEPT: newly included table is replicated'
+);
+$result =
+  $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM sch1.tab_published");
+is($result, qq(6),
+ 'ALTER ... SET TABLES IN SCHEMA EXCEPT: now-excluded table is not replicated'
+);

Instead of having to keep track of the running totals IMO it might be
simpler if you just did "SELECT ... WHERE a = 7;" then the answer
would be just 1 or 0 rows.

~~~

18.
+# SET without EXCEPT: clears the except list; both tables are now published.
+# tab_published will be re-synced because REFRESH removed its entry when it was
+# excluded.  Truncate the subscriber copy beforehand so the re-sync produces
+# a predictable count: publisher has 7 rows (6 original + INSERT(7)), so the
+# subscriber ends up with 7 after re-sync, then 8 after INSERT(8).

This is similar to my previous comment. There is lots of tricky
commentary here because you are trying to keep track of running totals
of rows.

I think most of this might not be needed if you changed the checks to
do ""SELECT ... WHERE a = 8;", so then you are just expecting row
count to be 1 for both tables.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Thu, 28 May 2026 16:56:56 +0530

> Here are some review comments for patch v6-0001.

Thanks for the review. All comments are addressed in v7. Please find
responses below for a few of the comments.

> relate to the 'ancestors'?

Updated the function comments to explain except_pubids.

> even assign/check aschemaPubids?

except_pubids represents the set of publication OIDs from which the
root relation has been explicitly excluded via EXCEPT (TABLE ...).
But yes, we can check it before computing schemaPubids. Fixed.

> Fully qualified 'targetrel' in the first error, but not in the second? Is it OK?

IMO, we should use fully qualified names in both cases. Though the
second error is not part of this patch, I’ve updated it as well.
Please let me know if you think otherwise.

> src/backend/replication/pgoutput/pgoutput.c
...
> Can we standardize on some common names, to make all the code more consistent?

I have now used the "except_" prefix consistently for all names to
avoid confusion. Also updated the naming in all places as below:
except_rel_names: parse-level table names in EXCEPT
except_rels: internal relations in the EXCEPT list (opened/accessed)
except_relids: list of relation OIDs wherever used
except_pubids: publication OIDs from which a given relation (or its
root) is excluded

> found other examples in PG source code also not freeing...

Sorry, this was mistakenly removed in v6 while cleaning up the code,
and I did not verify it against the older version. Fixed now.

> Query_for_list_of_tables?

For this case, I don't think it's really possible to keep suggesting
after a comma. Even if we handle a fixed number of comma-separated
entries, the same problem reappears with the next entry.
Query_for_list_of_tables_in_schema needs a correct schema reference,
but with each comma the relative offset changes, so there is no single
fixed prev*_wd that can reliably point to the schema across all
entries.

But I see, the current call to Query_for_list_of_tables is clearly
incorrect here. I have now suppressed suggestions after a comma,
instead of showing incorrect suggestions.
Thoughts?

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Thu, 28 May 2026 16:58:10 +0530

> thread and patched/fixed asap, then your patch should just make small

Agree. I'll make a separate patch for it and start a thread.

> stmt->action == AP_AddObjects/SetObjects ?

Agree, fixed.

> Huh? It seems contrary to the function comment that was also talking

Corrected the function comments. It is for only schema publications.

> 'AlterPublicationSchemaExceptTables'?

Done. Renamed as suggested.

> delegated from) the AlterPublicationSchemas?

+1, fixed.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Thu, 28 May 2026 16:58:31 +0530

> that was all mentioned already in the earlier "third variant"

Right. it seems repeating. Removed "SET TABLES IN SCHEMA" related info.

> possible confusion about it being supported. Isn't it better to just

Okay, that makes sense. Fixed.

> EXCEPT, but IIRC I already reported that in a previous review.

Yes, we can handle this in a separate patch.

> + * false, tripping the assertion.

The assertion is Assert(GetPublication(pubid)->alltables ||
is_schema_publication(pubid)) in GetExcludedPublicationTables().
I’ve trimmed the comment a bit, as it felt slightly over-explained.
~~~~

Please find the updated patch-set v7 attached.

--
Thanks,
Nisha

---

From: Peter Smith <[email-masked]>
Date: Fri, 29 May 2026 15:31:01 +1000

Hi Nisha.

Some review comments for patch v7-0001.

======

1.
- else
+ else if (!list_member_oid(except_pubids, puboid))
  {
  aschemaPubids = GetSchemaPublications(get_rel_namespace(ancestor));

IIUC this `except_pubids` and `puboid` are not changing, so you do not
need to be doing this member check every loop iteration. Is it better
to assign some variable up-front?

e.g.
bool check_schemas = !list_member_oid(except_pubids, puboid);

~~~

2.
+ if (is_except)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("relation \"%s\" cannot be added because it is excluded from
publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel),
+ pub->name)));

I am unsure about the changed wording from "table" to "relation". IIUC
we say "relation" when it could be either a table or a sequence. So
maybe "table" is correct for your patch;l OTHOH this should change to
"relation" by Shlok's EXCEPT SEQUENCE patch [1].

~~~

3.
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("relation \"%s\" is already member of publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel), pub->name)));

IMO making everything fully qualified like this would be a good
change, but doing it here perhaps does not belong in your patch. I
have resurrected this question in the other thread [2], which would
affect not only this statement. but many others. Please post your
opinion about this on that other thread.

======

ObjectsInPublicationToOids:

4.
 static void
 ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
-    List **rels, List **exceptrels, List **schemas)
+    List **rels, List **except_rel_names, List **schemas)

Is `except_rel_names` an accurate name? IMO it makes it sound like
it's a list of char* names, but IIUC that is not the case; aren't
these PublicationTable objects? Would something like
`except_pubtables` be more correct?

~~~

CreatePublication:

5.
- List    *exceptrelations = NIL;
+ List    *except_rel_names = NIL;

Same doubts about this `except_rel_names` variable name.

~~~

AlterPublication:
6.
  List    *relations = NIL;
- List    *exceptrelations = NIL;
+ List    *except_rel_names = NIL;

Same doubts about this `except_rel_names` variable name.

======

get_rel_sync_entry:

7.
+ if (am_partition)
+ {
+ List    *part_ancestors = get_partition_ancestors(relid);
+
+ root_relid = llast_oid(part_ancestors);
+ list_free(part_ancestors);

I think just call this `ancestors` (not `part_ancestors`) for
consistency with other code in the same function.

======

...
> instead of showing incorrect suggestions.

8.
REPLY: Yeah, I don't have any good ideas how to fix this, or if a fix
is even possible, but I agree that doing nothing is better than doing
the wrong thing.

~~~

9.
BTW, the current code is not able to handle multiple schemas.

So, this works:
test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA myschema <TAB>
EXCEPT ( TABLE  WITH (

but, this doesn't do anything:
test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema <TAB>

======
[1] Shlok EXCEPT -
https://www.postgresql.org/message-id/flat/CAHut%2BPsUrYmbZ996ZybjMWvpW_ufXB8WM94pdvAPyzQpoe%2BHRA%40mail.gmail.com#579d9b99c4f620602085cc59ff0e2b7d
[2] schema-qualified messages -
https://www.postgresql.org/message-id/CAHut%2BPvWoOyLKFb627Ch%2BXg3TYHuHdaOZ_XmxYgKVYdOzpqFsw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Fri, 29 May 2026 18:24:16 +1000

Hi Nisha.

Some review comments for patch v7-0002.

======

1.
+static void AlterPublicationSchemas(AlterPublicationStmt *stmt,
+ HeapTuple tup, List *schemaidlist,
+ List *except_rel_names);
+static void AlterPublicationSchemaExceptTables(AlterPublicationStmt *stmt,
+    HeapTuple tup,
+    List *except_rel_names,
+    List *schemaidlist);

Maybe the same doubts about the `except_rel_names` parameter/variable
are continued into this patch. See patch 0001 where I first queried
this.

======

2.
+ 'CREATE PUBLICATION pub12' => {
+ create_order => 50,
+ create_sql =>
+   'CREATE PUBLICATION pub12 FOR TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table);',
+ regexp => qr/^
+ \QCREATE PUBLICATION pub12 WITH (publish = 'insert, update, delete,
truncate');\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ },
+
+ 'ALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table)'
+   => {
+ regexp => qr/^
+ \QALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE ONLY test_table, TABLE ONLY test_second_table);\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+   },

I found those hard to read at first. How about just changing the test
title of the ALTER parts

BEFORE
+ 'ALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table)'
SUGGESTION
+ 'CREATE PUBLICATION pub12 test continues ...'

(2 places like this)

======
src/test/regress/expected/publication.out

3.
+-- DROP TABLES IN SCHEMA also removes associated EXCEPT entries
+ALTER PUBLICATION testpub_alter_except DROP TABLES IN SCHEMA pub_test;
+\dRp+ testpub_alter_except
+                                                       Publication
testpub_alter_except
+          Owner           | All tables | All sequences | Inserts |
Updates | Deletes | Truncates | Generated columns | Via root |
Description
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t
     | t       | t         | none              | f        |
+Except tables:
+    "pub_test.testpub_tbl_s1"
+

Isn't this showing a BUG, because after the DROP the "Except tables"
are still listed.

~~~

4.
+-- ADD: unqualified name is implicitly qualified with the schema, not public
+ALTER PUBLICATION testpub_alter_except ADD TABLES IN SCHEMA pub_test
EXCEPT (TABLE testpub_tbl_s2);
+\dRp+ testpub_alter_except
+                                                       Publication
testpub_alter_except
+          Owner           | All tables | All sequences | Inserts |
Updates | Deletes | Truncates | Generated columns | Via root |
Description
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t
     | t       | t         | none              | f        |
+Tables from schemas:
+    "pub_test"
+Except tables:
+    "pub_test.testpub_tbl_s1"
+    "pub_test.testpub_tbl_s2"
+

Isn't this showing the same BUG as the previous test, because "s1" is
still in the "Except tables" list, even though it was not part of the
EXCEPT.

~~~

5.
+-- ADD: multiple excepted tables using unqualified names
+ALTER PUBLICATION testpub_alter_except DROP TABLES IN SCHEMA pub_test;
+ALTER PUBLICATION testpub_alter_except ADD TABLES IN SCHEMA pub_test
EXCEPT (TABLE testpub_tbl_s1, testpub_tbl_s2);
+ERROR:  relation "pub_test.testpub_tbl_s1" cannot be added because it
is excluded from publication "testpub_alter_except"

Isn't this showing the same BUG again, because "s1" was already in the
EXCEPT list when it should not be.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Sat, 30 May 2026 10:02:23 +0530

> Some review comments for patch v7-0001.

Thanks for the review.

> "relation" by Shlok's EXCEPT SEQUENCE patch [1].

Okay, makes sense, changed back to "table".

> opinion about this on that other thread.

That is fine with me. I've reverted the change from my patch.

> `except_pubtables` be more correct?

Yes these are PublicationTable objects, changed the name as sugegsted.

> test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema <TAB>

I think the above preserves the existing behavior. Currently, we do
not suggest "WITH (" after the second schema onwards. To support this
properly, we would also need to handle "WITH (" suggestions for
subsequent schema entries.

I’ve created a top-up patch (patch-002) for this. I can merge it if we
want to change the current behavior. Let me know your thoughts.
~~~~

Attached is the updated v8 patch set.
Addressed all of the above comments, along with the patch v7-0002
comments from [1]. Patch-0003 has also been updated accordingly.

[1] https://www.postgresql.org/message-id/CAHut%2BPuhL7Xj8UAK0yBmbbDsCC9xvRVmreCC_yxr%2BbMfc-dt5g%40mail.gmail.com

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Sat, 30 May 2026 10:02:41 +0530

> Some review comments for patch v7-0002.

Thanks for the review. All comments are addressed in v8. Please find
responses below for a few of the comments.

> + 'CREATE PUBLICATION pub12 test continues ...'

I don't see any existing "..test continues..." pattern, so I changed it as -
'CREATE PUBLICATION pub11 - ADD TABLES IN SCHEMA EXCEPT dump'

Thoughts?

> Updates | Deletes | Truncates | Generated columns | Via root |

DROP handling is part of Patch-0003, so the DROP-related tests belong
there. I had added the test here only to verify the ADD scenarios, but
I agree that it makes the coverage confusing and incorrect in its
current placement.

I’ve now corrected the tests accordingly.

--
Thanks,
Nisha

---

From: Peter Smith <[email-masked]>
Date: Mon, 1 Jun 2026 17:24:07 +1000

Hi Nisha.

Review comments for v8-0001 and v8-0002.

======
git apply gives warnings.

1.
git apply ../patches_misc/v8-0001-Support-EXCEPT-clause-for-schema-level-publicatio.patch
../patches_misc/v8-0001-Support-EXCEPT-clause-for-schema-level-publicatio.patch:176:
space before tab in indent.
                                         errmsg("relation \"%s\" is
already member of publication \"%s\"",
warning: 1 line adds whitespace errors.

======

On Sat, May 30, 2026 at 2:32 PM Nisha Moond <[email-masked]> wrote:
...
> want to change the current behavior. Let me know your thoughts.

2.
Some scenarios are improved, but others do not work (either newly
broken or maybe they have been?).

TBH, I am unsure if the added complexity of patch 0002 was worth it. I
am going to pass on this for now and wait for other opinions.

e.g.

Good: (suggests schemas to use)
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA
CURRENT_SCHEMA      information_schema  myschema            public

Bad: (does not suggest more schema to use)
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public, <TAB HERE>

Good: (completes names of known schema)
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA my <TAB HERE>
becomes
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA myschema

Bad: (does not complete names of known schema)
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public, my <TAB HERE>

Good: (suggest EXCEPT with single schema)
test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA myschema
EXCEPT ( TABLE  WITH (

Good: (suggest EXCEPT with multi schema)
test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema
EXCEPT ( TABLE  WITH (

Bad: (doesn't work if the FOR TABLE precedes TABLES IN SCHEMA)
test_pub=# CREATE PUBLICATION pub1 FOR TABLE mytab, TABLES IN <TAB HERE>

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Mon, 1 Jun 2026 17:26:14 +1000

Hi Nisha.

Some review comments for patch v8-0003.

======

AlterPublicationSchemaExceptTables:

1.
+ /*
+ * EXCEPT is not meaningful with DROP: dropping a schema from a
+ * publication already removes all its except entries via cascade, and
+ * there is no sensible interpretation of "drop only the except entry but
+ * keep the schema".
+ */

Is that backwards? I think you mean :

SUGGESTION
* Dropping a schema from a publication removes all its EXCEPT entries via
* cascade. The concept of "drop all schema tables from the publication EXCEPT
* these ones" is not supported.
======
src/bin/pg_dump/t/002_pg_dump.pl

2.
> 'CREATE PUBLICATION pub11 - ADD TABLES IN SCHEMA EXCEPT dump'

I've since found that there is a way to combine multiple regex within
a single test. Doing it like below is a cleaner way to write these
multi-statement tests.

SUGGESTION (note /xms instead of /xm)
    'CREATE PUBLICATION pub11' => {
        create_order => 50,
        create_sql =>
          'CREATE PUBLICATION pub11 FOR TABLES IN SCHEMA dump_test
EXCEPT (TABLE test_table);',
        regexp => qr/^
            \QCREATE PUBLICATION pub11 WITH (publish = 'insert,
update, delete, truncate');\E
            .*?
            \QALTER PUBLICATION pub11 ADD TABLES IN SCHEMA dump_test
EXCEPT (TABLE ONLY test_table);\E
            /xms,
        like => { %full_runs, section_post_data => 1, },
    },

(ditto for the pub12 test)

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Mon, 1 Jun 2026 17:28:53 +1000

Hi Nisha.

Some review comments for patch v8-0004.

======

1.
+ /* Collect OIDs of the desired new EXCEPT list. */
+ foreach_ptr(PublicationRelInfo, pri, rels)
+ {
+ newexceptrelids = lappend_oid(newexceptrelids,
+   RelationGetRelid(pri->relation));
+ }

Block braces {} not needed.

~~~

2.
+ if (!OidIsValid(proid))
+ continue; /* already gone */
+
+ ObjectAddressSet(obj, PublicationRelRelationId, proid);
+ performDeletion(&obj, DROP_CASCADE, 0);

SUGGESTION
if (OidIsValid(proid))
{
  ObjectAddressSet(obj, PublicationRelRelationId, proid);
  performDeletion(&obj, DROP_CASCADE, 0);
}

======

3.
I think you had used the SQL exactly as I previously suggested, but I
made a mistake:
It should say "SELECT count(*)" instead of "SELECT a".

So it returns either 0 or 1 row.

e.g. #1
$result =
  $node_subscriber->safe_psql('postgres',
    "SELECT count(*) FROM sch1.tab_excluded WHERE a = 7");
is($result, qq(1),
    'ALTER ... SET TABLES IN SCHEMA EXCEPT: newly included table is replicated'
);
$result =
  $node_subscriber->safe_psql('postgres',
    "SELECT count(*) FROM sch1.tab_published WHERE a = 7");
is($result, qq(0),
    'ALTER ... SET TABLES IN SCHEMA EXCEPT: now-excluded table is not
replicated'
);

e.g. #2
$result =
  $node_subscriber->safe_psql('postgres',
    "SELECT count(*) FROM sch1.tab_published WHERE a = 8");
is($result, qq(1),
    'ALTER ... SET TABLES IN SCHEMA (no EXCEPT): tab_published
replicated after except list cleared'
);
$result =
  $node_subscriber->safe_psql('postgres',
    "SELECT count(*) FROM sch1.tab_excluded WHERE a = 8");
is($result, qq(1),
    'ALTER ... SET TABLES IN SCHEMA (no EXCEPT): tab_excluded
replicated after except list cleared'
);

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Tue, 2 Jun 2026 14:26:18 +0530

Thanks for the review.

> broken or maybe they have been?).

I tested these scenarios on HEAD (without this patch), and most of
them already exist today. (See inline below).

> CURRENT_SCHEMA      information_schema  myschema            public

Not introduced by this patch; it is existing behavior.

> test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public, <TAB HERE>

Not introduced by this patch; it is existing behavior.

> test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA myschema

Not introduced by this patch; it is existing behavior.

> test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public, my <TAB HERE>

Not introduced by this patch; it is existing behavior.

> test_pub=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA myschema

This behavior is introduced by v8-0001.

> test_pub=# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema

This behavior is introduced by v8-0002.

> test_pub=# CREATE PUBLICATION pub1 FOR TABLE mytab, TABLES IN <TAB HERE>

Not introduced by this patch; it is existing behavior.
~~~

All of the above behavior is consistent with HEAD. Only the "EXCEPT
(TABLE" suggestions are introduced by patches 0001 and 0002.

I'm also not sure the added complexity is justified just for the
"EXCEPT (TABLE" suggestion, especially since suggestions after commas
are generally not supported in most existing cases.
I'll drop patch-0002 for now and we can revisit it later if others
have opinions on it.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Tue, 2 Jun 2026 14:26:49 +0530

> Some review comments for patch v8-0003.

Thanks for the

> * these ones" is not supported.

Fixed as suggested in v9.

> (ditto for the pub12 test)

Thanks for the suggestion. I've updated the test accordingly in v9.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Tue, 2 Jun 2026 14:27:24 +0530

Thanks for the review.

> Block braces {} not needed.

Fixed.

>   performDeletion(&obj, DROP_CASCADE, 0);

Fixed. A similar pattern was also used in PublicationDropSchemas(),
and I have fixed that as well.

> replicated after except list cleared'

Thanks for pointing that out; I overlooked your earlier suggestion.
I've now updated as suggested.

Attached is the updated v9 patch set.

--
Thanks,
Nisha

---

From: Zsolt Parragi <[email-masked]>
Date: Tue, 2 Jun 2026 14:21:48 -0700

Hello

In this simple script:

CREATE SCHEMA s;
CREATE TABLE s.a(i int);
CREATE TABLE s.b(i int);
CREATE PUBLICATION p FOR TABLES IN SCHEMA s EXCEPT (TABLE s.b);

SELECT * FROM pg_publication_tables WHERE pubname = 'p';
SELECT * FROM pg_get_publication_tables(ARRAY['p'], 's.b'::regclass);

Shouldn't the second select return an empty list? Currently it returns
one row for s.b.

And I think there's also a bug with CREATE PUBLICATION with IN SCHEMA
CURRENT_SCHEMA:

CREATE SCHEMA s1;
CREATE TABLE s1.a(i int);
CREATE TABLE public.x(i int);
CREATE TABLE public.onlypub(i int);   -- exists only in public, not in s1

CREATE PUBLICATION pn FOR TABLES IN SCHEMA s1 EXCEPT (TABLE public.x);
-- error out, good
SET search_path = s1, public;
CREATE PUBLICATION pn2 FOR TABLES IN SCHEMA s1 EXCEPT (TABLE onlypub);
-- error out, good
SET search_path = s1, public;
SELECT current_schema(); -- s1
CREATE PUBLICATION pc FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT
(TABLE public.x); -- doesn't error out
CREATE PUBLICATION pc4 FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT
(TABLE onlypub); -- also doesn't error out

---

From: Nisha Moond <[email-masked]>
Date: Fri, 5 Jun 2026 17:11:34 +0530

> Hello

Hi, thanks for the tests.

> Shouldn't the second select return an empty list? Currently it returns

Yes, it is a bug.
The issue was that pg_get_publication_tables() could not distinguish
between prexcept=true and prexcept=false.

A reference note: This worked fine for FOR ALL TABLES publications
because entries in pg_publication_rel always had prexcept=false, so
the presence of an entry implied the table was not published. With
non-ALL TABLES publications, a table can now have both prexcept=true
(explicitly excluded) and prexcept=false (excluded from a schema
publication).

Fixed.

> (TABLE onlypub); -- also doesn't error out

Yes, this is a bug. The CURRENT_SCHEMA case was missed when the patch
was updated to allow only immediate schema tables in the EXCEPT
clause.

Attached are v10 patches with fixes for both issues. I also added the
missing tab-completion for CURRENT_SCHEMA and added regression tests
in publication.sql covering both scenarios.

--
Thanks,
Nisha

---

From: Zsolt Parragi <[email-masked]>
Date: Sat, 6 Jun 2026 15:39:18 -0500

Hello!

Thanks, I can confirm the fixes work.

I did some more testing. I think I see two problems with ALTER TABLE
... SET SCHEMA:

1.

CREATE SCHEMA s;
CREATE SCHEMA other;
CREATE TABLE s.t(i int);
CREATE PUBLICATION p FOR TABLES IN SCHEMA s EXCEPT (TABLE s.t);
ALTER TABLE s.t SET SCHEMA other;
ALTER PUBLICATION p ADD TABLES IN SCHEMA other;
-- shouldn't s.t be there?
SELECT schemaname, tablename FROM pg_publication_tables WHERE
pubname='p' ORDER BY 1,2;

2.

CREATE SCHEMA s;
CREATE SCHEMA other;
CREATE TABLE s.t(i int);
CREATE PUBLICATION p FOR TABLES IN SCHEMA s EXCEPT (TABLE s.t);
ALTER TABLE s.t SET SCHEMA other;
ALTER PUBLICATION p DROP TABLES IN SCHEMA s;
-- should it still be there? it isn't without the alter set schema
SELECT pr.prrelid::regclass AS rel, pub.pubname, pr.prexcept
FROM pg_publication_rel pr JOIN pg_publication pub ON pub.oid=pr.prpubid;

---

From: Nisha Moond <[email-masked]>
Date: Tue, 9 Jun 2026 21:51:04 +0530

> I did some more testing. I think I see two problems with ALTER TABLE

Thanks for reporting this case.

The current patches did not handle schema changes for excluded tables.
Hou-san also reported the same design issue off-list to me.

After considering, I chose to follow behavior similar to existing FOR
ALL TABLES publications to handle schema-switch cases. Today, if a
table excluded via EXCEPT is dropped, the corresponding prexcept entry
is removed, and recreating a table with the same name does not
automatically restore the exclusion.

I applied the same principle to schema changes: once an excluded table
moves out of the schema, the exclusion is removed.

For example, consider the following cases:
CREATE PUBLICATION p FOR TABLES IN SCHEMA s1 EXCEPT (TABLE s1.t);

case-1: Table moves from s1 to s2:  ALTER TABLE s1.t SET SCHEMA s2;
-- The exclusion entry s1.t is removed.

case-2: Table moves back from s2 to s1:  ALTER TABLE s2.t SET SCHEMA s1;
-- The table is published again. The exclusion is not restored
automatically; the user must specify it again.

case-3: If s2 is also part of publication p
 -- The behavior remains the same. Moving the table between schemas
does not recreate the exclusion entry.

IOW, once the exclusion is broken by a schema move (similar to a
drop), it must be re-established explicitly by the user.

The attached patch implements this behavior. I've also updated the
docs to describe it.

I also considered two alternatives:
1) Reject the schema change: Error out if a table with a prexcept
entry is moved between schemas. This feels overly restrictive.
2) Make exclusions schema-aware: Add a prexceptschema column and store
the schema OID along with the exclusion entry. The exclusion would
only apply while the table remains in that schema, allowing it to be
restored automatically if the table moves back later. IMO it
complicates the design.

Thoughts?

> pubname='p' ORDER BY 1,2;

Do you mean other.t should be there?
This is now fixed. When t is moved to schema other, the exclusion is
removed and the table is published through p.

postgres=# SELECT schemaname, tablename FROM pg_publication_tables
WHERE pubname='p' ORDER BY 1,2;
 schemaname | tablename
------------+-----------
 other      | t

> FROM pg_publication_rel pr JOIN pg_publication pub ON pub.oid=pr.prpubid;

Fixed.
Now, when s.t is moved out of schema s, it is removed from publication
p's exclusion list. Later, if schema s is dropped from publication p,
the exclusion entries associated with s are removed as well, and s.t
is no longer present there.

Attached are the updated v11 patches.
Patch 001 has the changes discussed above; patches 002 and 003 are unchanged.

--
Thanks,
Nisha

---

From: Zsolt Parragi <[email-masked]>
Date: Tue, 9 Jun 2026 16:36:13 -0500

> Do you mean other.t should be there?

Yes, that was a typo in my example.

> moves out of the schema, the exclusion is removed.

I'm not that sure about the analogy. DROP TABLE is a destructive
operation, executing DROP TABLE and then CREATE TABLE won't
automatically bring back the data.

With this approach, two cheap ALTER TABLE ... SET SCHEMA statements
can clear an EXCEPT clause without the proper permissions.

But I'm not sure what's the best solution for this. The v11 approach
is at least more consistent than the previous behavior.

> entry is moved between schemas. This feels overly restrictive.

It is restrictive, but maybe it's the better solution? Or
alternatively, maybe it should require proper permissions to remove
the except clause?

Another thing that could improve this if we would print out a warning
that the statement caused a change in the publication? But then that's
also a question for the preexisting drop table case.

---

From: Peter Smith <[email-masked]>
Date: Wed, 10 Jun 2026 14:36:44 +1000

Hi Nisha.

Some review comments for v11-0001.

(I had no new review comments for v11-0002, v11-0003)

======
doc/src/sgml/ref/create_publication.sgml

1.
+     <para>
+      For <literal>FOR TABLES IN SCHEMA</literal> publications, the
+      <literal>EXCEPT</literal> clause is schema-scoped: the exclusion applies
+      only in the context of the schema to which it was attached.  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 moved back to
+      the original schema, the exclusion is not restored; the user must
+      re-establish it explicitly using <command>ALTER PUBLICATION</command>.
+      Dropping a table always removes it from the <literal>EXCEPT</literal>
+      list regardless of publication type.
+     </para>

1a.
I felt this should be moved up to be the 2nd paragraph of the "EXCEPT"
part. Subsequent information about
inheritance/partitions/multi-publications is common for both EXCEPTS.

~

1b.
All that info about "If a table..." seemed more relevant to ALTER
PUBLICATION than to CREATE PUBLICATION, so I didn't think we needed
those details here.

======

RemovePublicationExceptForRelation:

2.
+/*
+ * Remove any EXCEPT clause entries for a relation from schema publications.
+ * Called when a table changes schema (ALTER TABLE ... SET SCHEMA), so that
+ * a schema-scoped exclusion does not silently follow the table to its new
+ * schema.  FOR ALL TABLES publications are skipped because their EXCEPT
+ * clause is publication-scoped, not schema-scoped, so that exclusion should
+ * persist regardless of what schema the table is in.
+ */

Instead of saying "FOR ALL TABLES publications are skipped", rephrase
that to be something like: "This problem does not apply to FOR ALL
TABLES publications because..."

Anyway, I think you can remove that note from the function comment,
and instead put it here:
+ if (!is_schema_publication(pubid))
+ continue;

~~~

3.
+{
+ List    *pubids;
+ ListCell   *lc;
+ ObjectAddress obj;
+
+ pubids = GetRelationExcludedPublications(relid);
+
+ foreach(lc, pubids)

Using a `foreach_oid` loop might be tidier here.

======
src/backend/commands/tablecmds.c

4.
  table_close(classRel, RowExclusiveLock);
+
+ /*
+ * Remove any EXCEPT clause entries for this relation from schema
+ * publications.  A schema-scoped exclusion is no longer meaningful once
+ * the table moves to a different schema.
+ */
+ if (rel->rd_rel->relkind == RELKIND_RELATION ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ RemovePublicationExceptForRelation(RelationGetRelid(rel));

Should this code be put prior to the table_close, where the other
dependent stuff is also removed?

======

5.
+ /* For TABLES_IN_CUR_SCHEMA: leave except_tables for execution time */

Isn't this repeating exactly what you already said in the other
comment ("For TABLES_IN_CUR_SCHEMA the schema name is not yet
known...")?

======

6.
+-- test for EXCEPT clause with schema publication (bug: excluded
table was incorrectly returned)
+SELECT * FROM test_gpt(ARRAY['pub_schema_except'],
'gpt_test_sch.tbl_sch'); -- no result (excluded)
+SELECT * FROM test_gpt(ARRAY['pub_schema_except'],
'gpt_test_sch.tbl_sch2'); -- one row (included via schema)
+

Is that "(bug: ...)" comment necessary?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Wed, 10 Jun 2026 14:33:04 +0530

> can clear an EXCEPT clause without the proper permissions.

We already have similar behavior today. A non-publication owner can
run ALTER TABLE ... SET SCHEMA and remove a table from a FOR TABLES IN
SCHEMA publication without any warning or publication-level permission
check.

> alternatively, maybe it should require proper permissions to remove

My concern is that introducing a permission check only for the EXCEPT
case would create an inconsistency: one type of schema move affecting
publication behavior would require publication ownership, while
another would not.

That said, I'm okay with restricting schema changes for tables in an
EXCEPT list if others feel that's the right behavior. Let's wait for
feedback from others.

> also a question for the preexisting drop table case.

Right, As also mentioned above, ALTER TABLE changes that affect
publication membership currently do not emit any notice or warning, so
I'm not sure we need one here either.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Wed, 10 Jun 2026 15:56:42 +0530

> (I had no new review comments for v11-0002, v11-0003)

Thanks for the review. I've addressed all the comments as suggested
and attached the updated v12 patch set.

--
Thanks,
Nisha

---

From: Zsolt Parragi <[email-masked]>
Date: Wed, 10 Jun 2026 13:54:15 -0700

> Let's wait for feedback from others.

Yes, I think that's the best approach for this question.

> I'm not sure we need one here either.

My idea was to make this change both for existing cases and new cases
in this patch, so it would be consistent, but that wasn't exactly
clear in my previous email, sorry about that. Similarly to how cascade
reports additional objects being dropped.

---

From: Peter Smith <[email-masked]>
Date: Thu, 11 Jun 2026 16:47:17 +1000

Hi Nisha.

Some review comments for the v12 patches.

//////
v12-0001

======

1.
      <para>
       This clause specifies a list of tables to be excluded from the
-      publication.
+      publication. It can be used with <literal>FOR ALL TABLES</literal> or
+      <literal>FOR TABLES IN SCHEMA</literal>.
+     </para>
+     <para>
+      For <literal>FOR TABLES IN SCHEMA</literal> publications, the
+      <literal>EXCEPT</literal> clause is schema-scoped: the exclusion applies
+      only within the schema to which it was attached.  For
+      <literal>FOR ALL TABLES</literal> publications, the exclusion applies to
+      the table regardless of the schema it is in.
      </para>

For that 2nd para, I think it ought to avoid words like "attached"
when talking about tables because attaching has a special meaning.

Also, I don't really understand what the 2nd sentence is trying to
say. FOR ALL TABLES exclusions will exclude the *named* tables. Yes,
those named tables might be in any schema (by fully-qualifying the
name), but that's not the same as saying "regardless of the schema".

So, IMO we can throw away that 2nd sentence, and just keep the
simplified 1st one. Since what remains is now very short, probably
just combine both paragraphs.

SUGGESTION
This clause specifies a list of tables to be excluded from the
publication. It can be used with <literal>FOR ALL TABLES</literal> or
<literal>FOR TABLES IN SCHEMA</literal>. For <literal>FOR TABLES IN
SCHEMA</literal>, the exclusion applies only to tables in the schema
associated with the <literal>EXCEPT</literal>.

======

2.
+ if (OidIsValid(proid))
+ {

Variable `obj` can be declared right here; it's the only place using it.

//////////
v12-0002

======

1.
+     <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>

I think the sentence "If the table is subsequently moved back..." is
overkill, and does not need to be said. The prior info "the exclusion
is removed" already tells me the exclusion is gone, and I think is
reasonable to assume "removed" means that it is gone for good, with no
ambiguity that it might magically come back.

YMMV. Leave it as-is if you prefer.

//////////
v12-0003

No comments.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Amit Kapila <[email-masked]>
Date: Mon, 15 Jun 2026 11:38:16 +0530

> reports additional objects being dropped.

IIUC, the objects here we are talking about are removed because of
their dependency type AUTO at least the existing case of 'drop table'.
So, I think the current behavior suggested by Nisha sounds correct and
consistent with the pre-existing 'drop table' case. Also, we display
such case at DEBUG2 level, see following code in dependency.c:

if (extra->flags & (DEPFLAG_AUTO |
DEPFLAG_INTERNAL |
DEPFLAG_PARTITION |
DEPFLAG_EXTENSION))
{
/*
* auto-cascades are reported at DEBUG2, not msglevel.  We don't
* try to combine them with the regular message because the
* results are too confusing when client_min_messages and
* log_min_messages are different.
*/
ereport(DEBUG2,
(errmsg_internal("drop auto-cascades to %s",

We can consider displaying such a message for schema cases, if not
already there.

-- 
With Regards,
Amit Kapila.

---

From: Amit Kapila <[email-masked]>
Date: Mon, 15 Jun 2026 11:49:57 +0530

I feel it is okay to keep the proposed sentence to avoid any ambiguity
by the user to consider the schema-scope state is symmetric.

-- 
With Regards,
Amit Kapila.

---

From: Nisha Moond <[email-masked]>
Date: Mon, 15 Jun 2026 17:00:42 +0530

> We can consider displaying such a message for schema cases, if not

The DROP TABLE case is handled automatically via dependency cascade,
so we already get above DEBUG2 log. However, when a table is moved to
a different schema, no such log is emitted because schema changes are
outside the scope of dependency cascade processing.

In v13, I added a DEBUG2 message when a table is removed from the
exclusion list due to a schema change.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Mon, 15 Jun 2026 17:00:50 +0530

> by the user to consider the schema-scope state is symmetric.

Okay, I have kept this para as-it-is.

The other comments on v12-0001 from [1] have also been addressed.

Please find the updated v13 patches attached.

[1] https://www.postgresql.org/message-id/CAHut%2BPv-GA1oGa6%2Bnwn_5AVhBg8NuJThQVUzqhQPXJge49jnew%40mail.gmail.com

--
Thanks,
Nisha

---

From: Zsolt Parragi <[email-masked]>
Date: Mon, 15 Jun 2026 14:45:42 -0700

> exclusion list due to a schema change.

Thanks, the DEBUG2 message (and the v13 patch) looks good to me and
this way it is consistent with the preexisting case. I have only a
question about test coverage - should the tests include verification
of the ALTER TABLE  ... SET SCHEMA behavior?

---

From: Peter Smith <[email-masked]>
Date: Tue, 16 Jun 2026 11:21:51 +1000

> Okay, I have kept this para as-it-is.

This discussion about the impact of ALTER TABLE ... SET SCHEMA made me
wonder what happens for the existing PG19 case of FOR ALL TABLES
EXCEPT (TABLE ...)

It turns out to be quite different:

======

test_pub=# CREATE SCHEMA myschema;
CREATE SCHEMA
test_pub=# CREATE TABLE t1(A INT);
CREATE TABLE
test_pub=# CREATE TABLE t2(A INT);
CREATE TABLE
test_pub=# CREATE TABLE myschema.myt1(A INT);
CREATE TABLE
test_pub=# CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT (TABLE myschema.myt1);
CREATE PUBLICATION
test_pub=# \d myschema.myt1
               Table "myschema.myt1"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           |          |
Excluded from publications:
    "pub1"

test_pub=# \dRp+ pub1
                                                       Publication pub1
  Owner   | All tables | All sequences | Inserts | Updates | Deletes |
Truncates | Generated columns | Via root | Descri
ption
----------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------
------
 postgres | t          | f             | t       | t       | t       |
t         | none              | f        |
Except tables:
    "myschema.myt1"

test_pub=# ALTER TABLE myschema.myt1 SET SCHEMA public;
ALTER TABLE
test_pub=# \d myt1
                Table "public.myt1"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           |          |
Excluded from publications:
    "pub1"

test_pub=# \dRp+ pub1
                                                       Publication pub1
  Owner   | All tables | All sequences | Inserts | Updates | Deletes |
Truncates | Generated columns | Via root | Descri
ption
----------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------
------
 postgres | t          | f             | t       | t       | t       |
t         | none              | f        |
Except tables:
    "public.myt1"

======

This experiment shows that moving the table did *not* remove the exclusion.

It is kind of "explainable" in hindsight because the exclusion is by
table OID, not name, so it follows the table around when it is moved.
I don't think this is what a user would expect, given that they
explicitly asked to exclude it from a different schema.

Is it a PG19 exclusion bug?

Is it behaviour that needs more documenting?

~

IMO it seemed like a bug of the PG19 FOR ALL TABLES EXCEPT, because it
is the opposite of what the new FOR TABLES IN SCHEMA EXCEPT patch
does:
"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;"

Thoughts?

======
Kind Regards,
Peter Smith.
Fujitsu Australia.

---

From: Amit Kapila <[email-masked]>
Date: Tue, 16 Jun 2026 10:05:51 +0530

>  Column |  Type   | Collation | Nullable | Default

No, I don't think this is a bug for PG19 and the new behaviour for
PG20 is intentional and required because in this case, the EXCEPT
clause is schema-scoped, so once schema is changed, the table should
be removed from the exclusion list. OTOH, in PG19, the exclusion list
follows the table-level exclusion based on its OID as we can see in
the example provided by you. I think we can consider adding a line for
this in docs if you and others feel that such explicit mention can
avoid ambiguity around this. How about something like the following as
a separate para in EXCEPT clause description: Once a table is
excluded, the exclusion applies to that table itself, regardless of
its name or schema. Renaming the table or moving it to another schema
with <command>ALTER TABLE ... SET SCHEMA</command> does not cancel the
exclusion."

-- 
With Regards,
Amit Kapila.

---

From: Peter Smith <[email-masked]>
Date: Tue, 16 Jun 2026 16:36:10 +1000

> with <command>ALTER TABLE ... SET SCHEMA</command> does not cancel the

OK. The explanatory text LGTM.

What's the next step? Should I create a new thread/patch to deal with this?

======
Kind Regards.
Peter Smith
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Tue, 16 Jun 2026 18:23:25 +1000

Some review comments for v13-0001.

======

1.
      <para>
       Marks the publication as one that replicates changes for all tables in
       the specified list of schemas, including tables created in the future.
+      Tables listed in the <literal>EXCEPT</literal> clause for a given schema
+      are excluded from the publication.
      </para>

Given Amit's suggestion [1] to modify the EXCEPT text for FOR ALL
TABLES, perhaps there also needs to be an equivalent note for ALL
TABLES IN SCHEMA. Maybe wait to see what happens [1], then you can use
similar wording.

======

2.
+/*
+ * Remove any EXCEPT clause entries for a relation from schema publications.
+ * Called when a table changes schema (ALTER TABLE ... SET SCHEMA), so that
+ * a schema-scoped exclusion does not silently follow the table to its new
+ * schema.
+ */
+void
+RemovePublicationExceptForRelation(Oid relid, Oid oldNspOid, Oid newNspOid)
+{

There's nothing about that function name to indicate it is only for
SCHEMA publications.
There must be a better -- e.g. 'MaybeRemoveExclusionFromSchemaPub', or
whatever...

~~~

3.
+ ereport(DEBUG2,
+ errmsg_internal("auto-drop exclusion of table %s from publication
%s: table moved to schema %s",
+ quote_qualified_identifier(get_namespace_name(oldNspOid),
+    get_rel_name(relid)),
+ get_publication_name(pubid, false),
+ quote_identifier(get_namespace_name(newNspOid))));

Even though this is a debugging message, for consistency, we might as
well quote everything the same as normal messages do:

"auto-drop exclusion of table \"%s\" from publication \"%s\": table
moved to schema \"%s\""

======
[1] https://www.postgresql.org/message-id/CAA4eK1J_QP8CXCshaCy_01ALQQrDSzoTHfXKcAbjiEvV-RkOOw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Tue, 16 Jun 2026 16:14:09 +0530

> TABLES IN SCHEMA. Maybe wait to see what happens [1], then you can use

Okay, noted. Let's first wait for the conclusion on this.

> There must be a better -- e.g. 'MaybeRemoveExclusionFromSchemaPub', or

I've renamed it to RemoveSchemaPubExceptForRel to keep it consistent
with the neighboring function names. Let me know if it works.

Fixed.
~~~

Also added a test as per Zsolt's suggestion at [1].
I moved all documentation changes into a separate 0004 patch to make
doc review easier.

Please find the attached v14 patches.

[1] https://www.postgresql.org/message-id/CAN4CZFPZnckA9-MPt6xj1QsRQTVKs3ZoKgdZ%2BQG1xa%3D%3DUD46Xw%40mail.gmail.com

--
Thanks,
Nisha

---

From: Amit Kapila <[email-masked]>
Date: Tue, 16 Jun 2026 16:38:03 +0530

> What's the next step? Should I create a new thread/patch to deal with this?

WFM.

-- 
With Regards,
Amit Kapila.

---

From: Peter Smith <[email-masked]>
Date: Wed, 17 Jun 2026 11:53:40 +1000

Hi Nisha.

Some review comments for v14-0001.

======

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.

////////////////////

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.

~~~

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.

~~~

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.

~

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.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Wed, 17 Jun 2026 13:38:10 +1000

> > What's the next step? Should I create a new thread/patch to deal with this?

Done. See new thread/patch [1].

======
[1] https://www.postgresql.org/message-id/flat/CAHut%2BPvQ5BqnawCQd6r1tqqd%2BiAJC-CuRY8wscuXSrpHGUzofA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Thu, 18 Jun 2026 12:05:37 +0530

> -- 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.

okay, retained "cleared".

Done.

> 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.

> unless you say otherwise.

Fixed.
---

Attached are the updated patches v15.

--
Thanks,
Nisha

---

From: Peter Smith <[email-masked]>
Date: Mon, 22 Jun 2026 14:16:17 +1200

Hi Nisha.

No further comments for patches v15-0001,0002,0003.

But, here are some comments for v15-0004.

======

1.
+     <para>
+      explicitly using <command>ALTER PUBLICATION</command>.
+     </para>

IIRC the recent commit 77b6dd9 that explained about moving tables to
other schemas, was only on the CREATE PUBLICATION page; not on the
ALTER PUBLICATION page.

So either:
(a) maybe this entire paragraph is not needed, or
(b) keep it, but then the behaviour for FOR ALL TABLES also needs to
be described here

======

2.
Once a table is excluded, the exclusion applies to that table
regardless of its name or schema. Renaming the table or moving it to
another schema using ALTER TABLE ... SET SCHEMA does not remove the
exclusion.

~~

The recent commit 77b6dd9 added the above text to the CREATE
PUBLICATION page. But, that was correct only for "FOR ALL TABLES" --
after your patch we can't say that unconditionally anymore. Also,
since that is mentioned on the CREATE PUBLICATION page, I guess we
need to say something similar about ALL TABLES IN SCHEMA ... EXCEPT in
the same paragraph.

So, maybe something like below:

SUGGESTION
Once a table is excluded under <literal>FOR ALL TABLES</literal>, the
exclusion applies to that table, the exclusion applies to that table
regardless of its name or schema. Renaming the table or moving it to
another schema using <command>ALTER TABLE ... SET SCHEMA</command>
does not remove the exclusion. However, for <literal>FOR TABLES IN
SCHEMA</literal>, because the <literal>EXCEPT</literal> is
schema-scoped, moving a schema-excluded table to another schema does
remove the exclusion.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Nisha Moond <[email-masked]>
Date: Mon, 22 Jun 2026 18:19:41 +0530

> schema-scoped, moving a schema-excluded table to another schema does

Based on the changes in commit 77b6dd9, I re-analyzed the pointed-out paragraph.

On reconsideration, I think the schema-move behavior is really a
property of EXCEPT itself rather than ALTER PUBLICATION. So we can
keep it only in the CREATE PUBLICATION documentation and drop it from
ALTER PUBLICATION to avoid duplication.
We already have a cross-reference in ALTER PUBLICATION: "See CREATE
PUBLICATION for further details on the semantics of EXCEPT", so users
still have a clear path to the detailed behavior description.
I modified your suggestion slightly to keep the information from the
removed paragraph intact.

Attached updated patches v16. No changes in 0001 to 0003.

--
Thanks,
Nisha