Re: Skipping schema changes in publication

On Fri, Feb 27, 2026 at 12:10 PM vignesh C <vignesh21@gmail.com> wrote:
>
> Rest of the comments are addressed in the attached v51 version.

Thanks for the patches, I am still testing the patches, but sharing a
few initial review comments.

1)  v51-0001: Bug in testcase - /t/037_rep_changes_except_table.pl
The test_except_root_partition() is invoked twice with values true and
false. However, from the logs it appears that both executions use
publish_via_partition_root = 1.
LOG output:
  138 2026-02-27 12:26:51.167 IST client backend[72317]
037_rep_changes_except_table.pl LOG:  statement: CREATE PUBLICATION
tap_pub_part FOR ALL TABLES EXCEPT TABLE (sch1.t1) WITH
(publish_via_partition_root = 1);
  ...
  ...
  238 2026-02-27 12:26:51.600 IST client backend[72369]
037_rep_changes_except_table.pl LOG:  statement: CREATE PUBLICATION
tap_pub_part FOR ALL TABLES EXCEPT TABLE (sch1.t1) WITH
(publish_via_partition_root = 1);

It seems $pubviaroot is assigned using the argument count instead of
the argument value, which causes both runs to behave the same.
Fix is to replace the assignment as:
  27 -  my $pubviaroot = @_;
  27 +  my ($pubviaroot) = @_;
~~~

2) v51-0001: File: src/backend/commands/tablecmds.c

+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot attach table \"%s\" as partition because it is
referenced in publication \"%s\" EXCEPT clause",
+    RelationGetRelationName(attachrel), pubnames.data),
+ errdetail("The publication EXCEPT clause cannot contain tables that
are partitions.");
+ errhint("Remove the table from the publication EXCEPT clause before
attaching it."));
+ }

In the ereport() call, there appears to be a small typo. A comma
should follow errdetail() instead of a semicolon.
~~~

3) v51-0002: File: src/backend/commands/publicationcmds.c
+
+ /* Check that user is allowed to manipulate the publication tables. */
+ if (excepttables && !pubform->puballtables)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("publication \"%s\" is defined as NON FOR ALL TABLES",
+    NameStr(pubform->pubname)),
+ errdetail("EXCEPT Tables cannot be added to or dropped from non FOR
ALL TABLES publications."));
 }

It seems the check is for both SET and DROP cases, but this patch is
only for SET. Also, I find it a bit confusing, could be made clearer.
Suggestion:
- errmsg("publication \"%s\" is defined as NON FOR ALL TABLES",
+ errmsg("publication \"%s\" is not defined as FOR ALL TABLES",
     NameStr(pubform->pubname)),
- errdetail("EXCEPT Tables cannot be added to or dropped from non FOR
ALL TABLES publications."));
+ errdetail("EXCEPT TABLE cannot be used with publications that are
not defined as FOR ALL TABLES."));
~~~

4) v51-0002 and v51-003:
File: doc/src/sgml/ref/alter_publication.sgml
...
-   remove one or more tables/schemas from the publication.  Note that adding
-   tables/schemas to a publication that is already subscribed to will
require an
+   except tables/tables/schemas in the publication with the specified list; the
+   existing except tables/ tables/schemas that were present in the publication
...
4a) After adding “except tables” as a third option, the slash based
formatting may be slightly confusing. It may be clearer to write as:
... the existing except tables, tables, or schemas that were present in ....
... The <literal>DROP</literal> clauses will remove one or more except
tables, tables, or schemas from the publication....

If the existing slash format is retained, there is a typo in "except
tables/ tables/schemas", extra space after tables/.

4b) Should we add examples for SET/DROP EXCEPT TABLE as well?
~~~

5) Inconsistency in EXCEPT TABLE syntax
In CREATE PUBLICATION, the syntax requires parentheses, for example
EXCEPT TABLE (t1, t2), whereas in SET and DROP it is accepted only
without parentheses, like - EXCEPT TABLE t1, t2;

Should we keep the syntax consistent across all commands?
~~~

--
Thanks,
Nisha