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

---

From: Peter Smith <[email-masked]>
Date: Tue, 23 Jun 2026 11:40:47 +1200

On Tue, Jun 23, 2026 at 12:49 AM Nisha Moond <[email-masked]> wrote:
...

Hi Nisha.

Thanks for all the changes. I don't have any more review comments at this time.

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

---

From: shveta malik <[email-masked]>
Date: Tue, 30 Jun 2026 15:46:58 +0530

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

Thanks Nisha. I have resumed review of patch. I have not goen through
complete patch yet, but please find a few initial comments:

1)
I get this during compilation:

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

2)
I am checking the flow:
get_rel_sync_entry-->GetTopMostAncestorInPublication

@@ -2267,7 +2290,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)

  ancestor = GetTopMostAncestorInPublication(pub->oid,
     ancestors,
-    &level);
+    &level,
+    except_pubids);

a)
For what all publications, flow will come to above point. I guess it
can come for a explict table pub or a schema pub. For a schmea pub,
except_pubids make sense but not for a explicit-table pub alone.
Please add a comment atop GetTopMostAncestorInPublication call here to
explain
the scenario adn then the need of except_pubids.

b)
We have got except_pubids in the begining of the function as:
except_pubids = GetRelationExcludedPublications(root_relid);

Now inside GetTopMostAncestorInPublication(), we are keeping
'except_pubids' as constant while moving through all the ancestors. We
are fetching GetRelationIncludedPublications and GetSchemaPublications
for each ancestor but are not computing
GetRelationExcludedPublications for each ancestor. It may not be
obvious (for many new readers) why 'except_pubids' is not fetched
again for each ancestor. Please add a comment there (perhaps because
EXCEPT list can not have any partition and can have only root)

3)
The error message below and its associated validation logic are
duplicated in three places:

"cannot appear in both the table list and the EXCEPT clause"

Do you think we could factor this into a small helper function and
reuse it everywhere? That would make the code more modular and
simplify future changes to validation or error messages.

thanks
Shveta

---

From: Nisha Moond <[email-masked]>
Date: Wed, 1 Jul 2026 15:37:01 +0530

> complete patch yet, but please find a few initial comments:

Thanks for review Shveta.

>  2232 |                                 List       *except_pubids = NIL;

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

> EXCEPT list can not have any partition and can have only root)

Added comments for both the cases.

Done.
~~~

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

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

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

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

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

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

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

Attached are the v17 patches.

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

---

From: Nisha Moond <[email-masked]>
Date: Wed, 1 Jul 2026 15:42:38 +0530

During tests, I found one behavior that needs discussion:

Consider a partition hierarchy where the root table is in schema s2, a
child partition is in schema s1, and the publication includes both
schemas but excludes only the root table.
Test case:
-- Root of the partition tree lives in s2
  CREATE TABLE s2.parent (id int) PARTITION BY LIST (id);

-- Child partition lives in s1
  CREATE TABLE s1.part PARTITION OF s2.parent FOR VALUES IN (1, 2, 3);

-- Publication covers BOTH schemas, but EXCEPTs only s2.parent
  CREATE PUBLICATION p FOR TABLES IN SCHEMA s1, TABLES IN SCHEMA s2
EXCEPT (TABLE s2.parent);

Currently, if the root is excluded, all child partitions are also
excluded, meaning s1.part is not published. However, s1 never excluded
'part' explicitly, so the behavior may not be intuitive.

We need to decide whether child partitions should always remain
excluded when the root is excluded (regardless of schema), or whether
schema membership should still allow publication.

I've added an XXX comment in pg_publication.c to track this until we
conclude on the expected behavior. Suggestions are welcome.

--
Thanks,
Nisha

---

From: shveta malik <[email-masked]>
Date: Wed, 1 Jul 2026 15:46:43 +0530

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

Nisha, this bug was introduced by your patch or does it exist in 'ALL
TABLES Except TABLE' too?

thanks
Shveta

---

From: shveta malik <[email-masked]>
Date: Wed, 1 Jul 2026 15:54:19 +0530

> schemas but excludes only the root table.

This is the exact scenario I had in mind when reviewing
'get_rel_sync_entry-->GetTopMostAncestorInPublication' flow, thus I
suggested adding comments about intentionally fetching the 'Exclude
list' only for the Root. I was about to test it in my next review
cycle.

The first thought is that if root is excluded, the partition should
also be excluded to maintain consistency with the 'ALL TABLES Except
ROOT' behaviour. But then it needs to be documented too. This is my
initial understanding, but I will think more when I resume the review.

---

From: Nisha Moond <[email-masked]>
Date: Thu, 2 Jul 2026 10:21:18 +0530

> TABLES Except TABLE' too?

The same case for 'ALL TABLES Except Table' was already handled
correctly as part of that feature (Ref
-RelationBuildPublicationDesc()). The bug was only in my patch where I
missed handling it earlier.

--
Thanks,
Nisha

---

From: shveta malik <[email-masked]>
Date: Thu, 2 Jul 2026 14:19:31 +0530

> missed handling it earlier.

Okay, then we are good.

A few comments:

1)
postgres=# alter publication pub1 add table s2.t1;
ERROR:  table "s2.t1" cannot be added because it is excluded from
publication "pub1"

It will be good to add hint in above error.

Suggestion:
ERROR: cannot add table "s2.t1" to publication "pub1"
DETAIL: Table "s2.t1" is currently in EXCEPT list
HINT: Change EXCEPT list using ALTER PUBLICATION ..SET TABLES IN
SCHEMA .. EXCEPT

Please rephrase above as needed. Look earlier such messages/hints
already added for ALL TABLES.

---

From: Amit Kapila <[email-masked]>
Date: Thu, 2 Jul 2026 14:33:47 +0530

On Wed, Jul 1, 2026 at 3:54 PM shveta malik <[email-masked]> wrote:
> ROOT' behaviour. But then it needs to be documented too.
>

+1. It is good to be consistent here with similar ALL TABLES case.
BTW, as we can't exclude the child table explicitly, this anyway seems
like the only option because otherwise, users won't have any way to
exclude the child table.

-- 
With Regards,
Amit Kapila.

---

From: Nisha Moond <[email-masked]>
Date: Fri, 3 Jul 2026 22:15:03 +0530

> already added for ALL TABLES.

Updated the error as suggested, with a slight change in DETAIL message.

> From the user's perspective, it seems slightly odd that if he has
> excluded a table from publication, he can not add it back using 'add
> table'. Was this point discussed earlier?
>

This hasn't been discussed yet.

I kept the current behavior, as IMO, it is hard to tell whether the
user is trying to re-include a table or whether the ADD is unintended
and should be blocked. To avoid ambiguity, I think EXCEPT entries
should be modified only through the EXCEPT (...) clause itself, such
as ALTER PUBLICATION ... ADD/SET ... EXCEPT(TABLE ...), or by omitting
EXCEPT in the SET case.

This would also keep the behavior of ADD TABLE and DROP TABLE aligned,
as both would operate only on the include list without touching the
EXCEPT list.
I think this also aligns with FOR ALL TABLES, where ADD TABLE is
disallowed, so the EXCEPT list can only be modified through SET ..
EXCEPT (...).

Thoughts?

> 2)
> Currently:
> postgres=# alter publication pub1 drop table s2.t1;
> s2.t1 is EXCEPT list here.
>
> Should we have below error for this case rather than above generic
> error, it will be more intuitive for user. Plus both add and drop
> table commands will have similar error-style for EXCEPT list.
> Thoughts?
>
> ERROR: cannot drop table "s2.t1" from publication "pub1"

Makes sense. Fixed.

> 3)
> Please see below test:
>
> --Setting Except list for pub1 which has all tables of s2 included:
> alter publication pub1 set tables in schema s2 EXCEPT ( TABLE t1, t2);
>
> --pg_publication_rel entries:
> pubname | schema_name | table_name | prexcept
> --------+-------------+------------+---------
> pub1    | s2          | t1         | t
> pub1    | s2          | t2         | t
>
>
> --Then added a partition:
> alter publication pub1 add table s2.tab_part_1_p2;
>
> --pg_publication_rel entries:
> pubname | schema_name |  table_name   | prexcept
> --------+-------------+---------------+---------
> pub1    | s2          | t1            | t
> pub1    | s2          | t2            | t
> pub1    | s2          | tab_part_1_p2 | f
>
>
> --Then changed pub1 to exclude tab_root (parent of tab_part_1_p2) as well:
>
> alter publication pub1 set tables in schema s2 EXCEPT ( TABLE t1, t2, tab_root);
>
> --pg_publication_rel entries:
> pubname | schema_name | table_name | prexcept
> --------+-------------+------------+---------
> pub1    | s2          | t1         | t
> pub1    | s2          | t2         | t
> pub1    | s2          | tab_root   | t
>
> Note theoutput above, tab_part_1_p2 is removed internally as part of
> earlier command since its parent tab_root is now excluded.
>
> --Now add parition back, it allowed to add it:
> alter publication pub1 add table s2.tab_part_1_p2;
>
> pg_publication_rel entries:
> pubname | schema_name |  table_name   | prexcept
> --------+-------------+---------------+---------
> pub1    | s2          | t1            | t
> pub1    | s2          | t2            | t
> pub1    | s2          | tab_root      | t
> pub1    | s2          | tab_part_1_p2 | f
>
>
> FOR ALL TABLES publicaiton, if root is excluded, there is no way
> publication can have parition included independently. Same should hold
> true here. Thoughts?

Right, based on the discussion at [1], since we follow "all partitions
are excluded if the root is excluded", a partition child should not be
allowed to be added when its partition root is already excluded from
the publication.
Fixed. Such an operation will now raise an error:

postgres=# alter publication pub2 add table t_part_p1;
ERROR:  cannot add table "public.t_part_p1" to publication "pub2"
DETAIL:  Partition ancestor "public.t_part" of table
"public.t_part_p1" is currently listed in the EXCEPT clause of the
publication.
HINT:  Change EXCEPT list using ALTER PUBLICATION ... SET TABLES IN
SCHEMA ... EXCEPT.
~~~

Attached are the updated patches v18.

[1] https://www.postgresql.org/message-id/CAA4eK1%2BNmQRjSHPLr0X8YBuC6joivFqgsY3_qJ5-RnuuwNGkRQ%40mail.gmail.com
--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Fri, 3 Jul 2026 22:15:14 +0530

> like the only option because otherwise, users won't have any way to

I've updated the related comments in v18 and also updated the CREATE
PUBLICATION docs to clarify this behavior.

--
Thanks,
Nisha

---

From: shveta malik <[email-masked]>
Date: Mon, 6 Jul 2026 08:41:53 +0530

> disallowed, so the EXCEPT list can only be modified through SET ..

Okay. Makes sense.

> HINT:  Change EXCEPT list using ALTER PUBLICATION ... SET TABLES IN

Okay, looks good.

thanks
Shveta

---

From: Peter Smith <[email-masked]>
Date: Mon, 6 Jul 2026 17:55:19 +1000

Hi Nisha.

Some review comments for v18.

//////////
Patch v18-0001
//////////

1.
+ /*
+ * A partition cannot be independently included when any of its partition
+ * ancestors is in this publication's EXCEPT list.  Doing so would leave
+ * the catalog inconsistent (root excluded, partition explicitly included)
+ * and the ancestor-cascade rule would silently override the include at
+ * replication time.  Individual partitions cannot appear in EXCEPT
+ * (rejected above), so this check only applies to non-EXCEPT adds.
+ */
+ if (!pri->except && targetrel->rd_rel->relispartition)

It might be tidier to deal with partitions in one place.

SUGGESTION
if (targetrel->rd_rel->relispartition)
{
  if (pri->except)
  {
    /* If in EXCEPT clause, must be root partitioned table */
    ...
  } else {
    /* If not in EXCEPT clause, check ancestors are not excluded */
    ...
  }
}

~~~

2.
+ errmsg("cannot add table \"%s\" to publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel),
+ get_publication_name(pubid, false)),
+ errdetail("Partition ancestor \"%s\" of table \"%s\" is currently
listed in the EXCEPT clause of the publication.",

"Partition ancestor .. of table" sounded odd.

How about:
errmsg:  cannot add partition \"%s\" to ...
errdetail: Ancestor \"%s\" of partition \"%s\" is currently ...

~~~

3.
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("cannot add table \"%s\" to publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel),
+ pub->name),
+ errdetail("Table \"%s\" is currently listed in the EXCEPT clause of
the publication.",

The table was already named in the errmsg. It seemed redundant to name
it again in the errdetail.

SUGGESTION
errdetail("The table is currently listed in the EXCEPT clause of the
publication.")

======

4.
+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("table \"%s\" cannot appear in both the table list and the
EXCEPT clause",
+    RelationGetQualifiedRelationName(pri->relation)));

Do we need to name the clauses in this errmsg? Users can see what they typed.

SUGGESTION
errmsg("table \"%s\" cannot be both published and excluded")

//////////
Patch v18-0003
//////////

PublicationDropTables:

1.
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("cannot drop table \"%s\" from publication \"%s\"",
+ RelationGetQualifiedRelationName(rel),
+ get_publication_name(pubid, false)),
+ errdetail("Table \"%s\" is currently listed in the EXCEPT clause of
the publication.",
+    RelationGetQualifiedRelationName(rel)),
+ errhint("Change EXCEPT list using ALTER PUBLICATION ... SET TABLES
IN SCHEMA ... EXCEPT.")));

1a.
(Same as above review comment for patch 0001).

The table was already named in the errmsg. It seemed redundant to name
it again in the errdetail.

SUGGESTION
errdetail("The table is currently listed in the EXCEPT clause of the
publication.")

~

1b.
Why hint to change the except list -- who says the EXCEPT list is wrong?

It seemed more like a user error to me if they are trying to drop
something from a publication when it is already excluded. Why does
this need any hint at all?

======

2.
I did not recognise the distinction in the test comments between
"fail:" and "error:".

~~~

3.
+-- error: ALTER PUBLICATION ... DROP TABLE on an excluded table is rejected
+-- with an EXCEPT-specific error; an EXCEPT entry is not a member.

I don't think you need to say "an EXCEPT entry is not a member." --
what does that mean, other than to redundantly say an excluded table
is a table that is excluded?

//////////
Patch v18-0004
//////////

1.
      <para>
       For partitioned tables, only the root partitioned table may be specified
       in <literal>EXCEPT</literal>. Doing so excludes the root table and
-      all of its partitions from replication. The optional
+      all of its partitions from replication, even any partition that lives
+      in a schema which is itself included in the publication.

BEFORE
For partitioned tables, only the root partitioned table may be
specified in EXCEPT. Doing so excludes the root table and all of its
partitions from replication, even any partition that lives in a schema
which is itself included in the publication.

SUGGESTION #1
For partitioned tables, only the root partitioned table may be
specified in EXCEPT. Excluding the root table automatically excludes
all of its partitions from replication, including those in schemas
that are otherwise included in the publication.

SUGGESTION #2 (TI doubt you even needed to mention about other schemas)
For partitioned tables, only the root partitioned table may be
specified in EXCEPT. Excluding the root table automatically excludes
all of its partitions from replication, including those that would
otherwise be included in the publication.

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

---

From: Peter Smith <[email-masked]>
Date: Tue, 7 Jul 2026 09:01:00 +1000

Hi Nisha.

Bug: The current patch is allowing conflicting exclusion lists...

======

Background:

It's always been possible for CREATE PUBLICATION to specify the same
schema or same table multiple times.

test_pub=# CREATE SCHEMA MYSCHEMA;
CREATE SCHEMA
test_pub=# CREATE TABLE T1(A INT);
CREATE TABLE
test_pub=# CREATE TABLE MYSCHEMA.T2(A INT);
CREATE TABLE
test_pub=# CREATE TABLE MYSCHEMA.T3(A INT);
CREATE TABLE

// same table multiple times is OK
test_pub=# CREATE PUBLICATION PUB3 FOR TABLE T1,T1,T1,T1;
CREATE PUBLICATION

// same schema multiple times is OK
test_pub=# CREATE PUBLICATION PUB1 FOR TABLES IN SCHEMA MYSCHEMA,
MYSCHEMA, MYSCHEMA;
CREATE PUBLICATION

~~~

But, this behaviour is only allowed if there are no specification
conflicts. e.g. You can't have multiple tables with column lists:

test_pub=# CREATE PUBLICATION PUB4 FOR TABLE T1,T1(A),T1,T1;
ERROR:  conflicting or redundant column lists for table "t1"

======

Bug:

IMO similar restrictions and a similar error message needs to happen
for clashing schema exclusions. Specifying the same schema multiple
times is fine, but it must be disallowed if there are conflicting
exclusion lists.

For example, the command below currently works, but it should not:

test_pub=# CREATE PUBLICATION PUB2 FOR TABLES IN SCHEMA MYSCHEMA
EXCEPT (TABLE T2), MYSCHEMA EXCEPT (TABLE T3), MYSCHEMA;
CREATE PUBLICATION

test_pub=# \dRp+ PUB2
                                                       Publication pub2
  Owner   | All tables | All sequences | Inserts | Updates | Deletes |
Truncates | Generated columns | Via root | Descri
ption
----------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------
------
 postgres | f          | f             | t       | t       | t       |
t         | none              | f        |
Tables from schemas:
    "myschema"
Except tables:
    "myschema.t2"
    "myschema.t3"

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

---

From: shveta malik <[email-masked]>
Date: Tue, 7 Jul 2026 08:39:48 +0530

> times is fine, but it must be disallowed if there are conflicting

+1. Good Catch.

---

From: shveta malik <[email-masked]>
Date: Tue, 7 Jul 2026 14:31:46 +0530

> HINT:  Change EXCEPT list using ALTER PUBLICATION ... SET TABLES IN

Thanks. Do you think we should restrict this case too? To me, this
seems like another variation of the above where conflicting entries
are given at the time of sub-creation itself. The root is excluded but
parittion is included .

postgres=# create publication pub1 for table s2.tab_part_1, tables in
schema s2 except (table tab_root);
CREATE PUBLICATION
postgres=# \dRp+
                                                      Publication pub1
 Owner  | All tables | All sequences | Inserts | Updates | Deletes |
Truncates | Generated columns | Via root | Description
--------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
 shveta | f          | f             | t       | t       | t       | t
        | none              | f        |
Tables:
    "s2.tab_part_1"
Tables from schemas:
    "s2"
Except tables:
    "s2.tab_root"

thanks
Shveta

---

From: shveta malik <[email-masked]>
Date: Tue, 7 Jul 2026 16:13:17 +0530

> Truncates | Generated columns | Via root | Description

Since alter is blocked for above case while create-sub is not, I had a
look at the new check added in check_publication_add_relation

a) The check introduced in this function for partitions/ancestors is
sufficient for cases where an except-list is already present, but not
for cases where we are in the process of adding one. Thus the
create-sub scenario above is not covered by this.

b) Also the new check in check_publication_add_relation() gets and
traverses complete list of ancestors

+ List    *ancestors = get_partition_ancestors(relid);
+
+ foreach_oid(ancestor, ancestors)

Isn't getting root (llast_oid(ancestors)) and checking that alone in
EXCEPT list will suffice? Or am I missing something?

thanks
Shveta

---

From: Amit Kapila <[email-masked]>
Date: Wed, 8 Jul 2026 11:58:00 +0530

On Tue, Jul 7, 2026 at 4:13 PM shveta malik <[email-masked]> wrote:
> Since alter is blocked for above case while create-sub is not,
>

If a combination is not supported in ALTER, we should disallow the
same in CREATE as well. So, +1 to restrict the case you reported.

> I had a

I think this should be possible because we allow only root tables in
the EXCEPT list. If so, and we are planning to change as per your
suggestion then we should add a comment as to why checking only last
is okay here.

-- 
With Regards,
Amit Kapila.

---

From: shveta malik <[email-masked]>
Date: Wed, 8 Jul 2026 14:22:31 +0530

Nisha, the patch (testing wise) is okay, but some code-optimization
might help. I am stuck on reveiw of GetTopMostAncestorInPublication()
and all its callers.

1)
caller 1: pub_rf_contains_invalid_column():

It is not understood why pub_rf_contains_invalid_column() does not
pass 'exceptPubids' to GetTopMostAncestorInPublication(). Perhaps a
comment will help. IIUC, a parittion (for which we are trying to
verify row-filter expression) can not co-exist in a subscription where
its ROOT is already excluded. This assumption will be true once you
address the issue of CREATE SUB (in [1]) sent in my previous email.
After that, this will still need a comment.

2)
Both caller 2 (get_rel_sync_entry) and caller 3
(is_table_publishable_in_publication) are trying to see if partition
should be considered as included in publication and thus are trying to
see if ROOT is excluded. But the implementation is very different.

The get_rel_sync_entry() does that by passing except_pubids to
GetTopMostAncestorInPublication(). This except_pubids was computed for
topmost ROOT of the parition in the caller and then it skips checking
pg_pub_namepspace if given pubid is part of except_pubids arguement.

While is_table_publishable_in_publication() does that by selecting
topmost ancestor incuded in a given pubid using
GetTopMostAncestorInPublication(), without consulting/computing
except_pubids. And then later checks if the ancestor returned by
GetTopMostAncestorInPublication() is in pg_publicaiton_rel with
'prexcept' true.

IMO, GetTopMostAncestorInPublication() should itself be inclusive of
logic where it filters out the table (does not return it as result) if
ROOT is excluded. And even we should not be passing an argument for
that (this is my initial thought). By making such a logic, we need not
to bother about all
the callers to see if caller has correct logic to deal with output of
GetTopMostAncestorInPublication(). Can you think on this line and
check the feasibility.

3)
I have yet to reveiw pub_contains_invalid_column, but I have a initial
understanding, that it will be similar to the case of
pub_rf_contains_invalid_column. Please verify on your end.

[1]: https://www.postgresql.org/message-id/CAJpy0uAuL7SqowL7T6HCELuVUr5ubwoGsjV1SQQRCXGme5L%3DpA%40mail.gmail.com

thanks
Shveta

---

From: shveta malik <[email-masked]>
Date: Wed, 8 Jul 2026 14:58:15 +0530

> GetTopMostAncestorInPublication(). Can you think on this line and

I thought more on this. Do you think we can do this?

GetTopMostAncestorInPublication() is already accepting a list of
ancestors in a ordered fashion, root at the end. We get root from this
ancestor list, check if root is in pg_publication_rel with
except=true. If so, we skip the rest of the logic and return
InvalidOid from GetTopMostAncestorInPublication().

The 'except_pubids' argument is not required. Callers need not to have
special logic to send this arguement or to have extra-processing on
output as done by is_table_publishable_in_publication() currently.
Let me know if I have missed anything.

thanks
Shveta

---

From: Nisha Moond <[email-masked]>
Date: Wed, 8 Jul 2026 20:50:25 +0530

> > times is fine, but it must be disallowed if there are conflicting

Thanks for the bug report. I've fixed it and attached the fix as a
separate top-up patch, v19-0002.

Until v18, schema qualification checks for EXCEPT tables were split
between two places:

1) TABLES IN SCHEMA <schema-name> (EXCEPT TABLE <table-name>);
 -- the schema qualification check was performed in gram.y, since all
required information was available during parsing.

2) TABLES IN SCHEMA CURRENT_SCHEMA (EXCEPT TABLE <table-name>);
 -- CURRENT_SCHEMA is resolved only at execution time, so the schema
qualification and eligibility checks were performed there.

The reported bug also affects cases such as:
CREATE PUBLICATION PUB2 FOR TABLES IN SCHEMA MYSCHEMA EXCEPT (TABLE
T2), CURRENT_SCHEMA EXCEPT (TABLE T3);
 -- where CURRENT_SCHEMA = myschema. Correct handling of this case
requires the checks to be performed after schema resolution, i.e., at
execution time. To reduce duplicate code, I moved the schema
qualification and eligibility checks for both cases to execution time
and removed the corresponding processing from gram.y.

Since this changes a fair amount of code, I've kept it as a separate
patch (v19-0002) to make review easier. Depending on the feedback, we
can optimize the implementation or move parts of it back into gram.y
before merging it into patch 0001.
~~~

Attached is the v19 patch set.
 -- Also Addressed all comments from Peter at [1] and Shveta at [2].
 -- The code optimization suggestions from Shveta at [3] are still
pending. I'm evaluating them and plan to address them in the next
version.
~~~

[1] https://www.postgresql.org/message-id/CAHut%2BPt-%2B6Q4-kmhqCZ2_FpaNRR%3DhaJk-GK1eEyTXcTTDnRnXA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAJpy0uAuL7SqowL7T6HCELuVUr5ubwoGsjV1SQQRCXGme5L%3DpA%40mail.gmail.com
[3] https://www.postgresql.org/message-id/CAJpy0uA_d-K-f94d-9Xw66Fqv7gdSGBYGzi9YcybtuEkYdQocQ%40mail.gmail.com

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Wed, 8 Jul 2026 20:52:17 +0530

...
> this need any hint at all?

Okay, I see your point. The intent of the hint was to guide users who
are actually trying to remove a table from the EXCEPT list by pointing
them to SET.

I've reworded it to:
HINT: Use ALTER PUBLICATION ... SET TABLES IN SCHEMA ... EXCEPT to
drop the table from the EXCEPT list.

Hopefully this is clearer and more helpful. Let me know if this works,
or if you still think no hint is the better option.

> I did not recognise the distinction in the test comments between

Fixed. I now use "fail" consistently throughout.

...

I lean towards Suggestion #1, as not publishing a table/partition when
its schema is published without any exception is a non-obvious case
and should be stated explicitly.

--
Thanks,
Nisha

---

From: Nisha Moond <[email-masked]>
Date: Wed, 8 Jul 2026 20:52:45 +0530

> same in CREATE as well. So, +1 to restrict the case you reported.

Fixed in v19.

> suggestion then we should add a comment as to why checking only last

Agreed; I implemented the suggested optimization and added comments
explaining why checking only the last element is sufficient.

--
Thanks,
Nisha

---

From: shveta malik <[email-masked]>
Date: Thu, 9 Jul 2026 14:53:34 +0530

> before merging it into patch 0001.

Nisha,

- * Also flattens except_tables from TABLES IN SCHEMA nodes into the list so
- * that ObjectsInPublicationToOids() sees them as top-level
EXCEPT_TABLE entries.
+ * The except_tables attached to TABLES IN SCHEMA nodes are left in place;
+ * ObjectsInPublicationToOids() qualifies names, validates schema membership,
+ * and flattens them once the schema OID is known.

Can you please explain what does 'flatten' mean here.

The flatten logic here is added by your patch001 only, it is not an
existing logic. To me (in my initial round of review), it seems okay
if we moved it to ObjectsInPublicationToOids(). The only thing is that
the one error (table in EXCEPT clause does not belong to schema) which
is possible to get at the parsing stage will now be emitted during
pub-creation stage. But since it will keep the entire logic of
except-list verification at one place and considering that
creat/alter-pub is not so frequent operation, I am fine with it. But
lets see if others have any comment here.

~~

I had a quick look at the changes:

+ * Include the ONLY-ness of this entry in its signature. EXCEPT
+ * (TABLE parent) excludes parent and its inheritance children,
+ * while EXCEPT (TABLE ONLY parent) excludes only parent itself.

I think this will unconditionally apply to paritioned tables as well
but for paritioned table, EXCEPT doc says that ONLY and * has no
effect, but here it shows the effect:

Thsi gives error:
postgres=# CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public EXCEPT
(TABLE ONLY tab_root), TABLES IN SCHEMA public EXCEPT (TABLE
tab_root);
ERROR:  42P17: schema "public" specified with conflicting EXCEPT lists
LOCATION:  ProcessSchemaExceptTables, publicationcmds.c:299

While this does not:
postgres=#  CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA public EXCEPT
(TABLE tab_root), TABLES IN SCHEMA public EXCEPT (TABLE tab_root);
CREATE PUBLICATION

But I also checked column-list errors and found that even if columns
are exact sames, we get error:

postgres=# CREATE PUBLICATION PUB4 FOR TABLE T1(i), T1(i);
ERROR:  42710: conflicting or redundant column lists for table "t1"
LOCATION:  OpenTableList, publicationcmds.c:2312

While this works:
postgres=# CREATE PUBLICATION PUB4 FOR TABLE T1, T1;
CREATE PUBLICATION

This makes me think that even introducing the smartness of identifying
ONLY in case of inherited tables and distinguishing it from
parititoned tables is not required. We can just change your error
message to 'conflicting or redundant EXCEPT lists for schmea "s1"' and
throw error in all cases where schmea with except list repeats without
actually comparing them. Does this sound reasonable?

thanks
Shveta