Re: Support EXCEPT for TABLES IN SCHEMA publications

Some review comments for v22*

======

1. GENERAL -- Consistent wording in messages

I found some examples of error messages that could be made more similar:

1a.
"modify" versus "change"

modify...

+ errhint("Use ALTER PUBLICATION ... SET TABLES IN SCHEMA ... EXCEPT
to modify the EXCEPT list.")));

versus (Change)

+ errhint("Change the EXCEPT clause using ALTER PUBLICATION ... SET
TABLES IN SCHEMA ... EXCEPT."));
+ errhint("Change the EXCEPT clause using ALTER PUBLICATION ... SET
TABLES IN SCHEMA ... EXCEPT.")));

+ errhint("%s", has_alltables_pub ?
+ _("Change the EXCEPT clause using ALTER PUBLICATION ... SET ALL
TABLES ... EXCEPT.") :
+ _("Change the EXCEPT clause using ALTER PUBLICATION ... SET TABLES
IN SCHEMA ... EXCEPT.")));

~

Shouldn't 1st one (from patch v22-0004) be identical to the others? e.g.:
Change the EXCEPT clause using ALTER PUBLICATION ... SET TABLES IN
SCHEMA ... EXCEPT.

~~~

1b.
"referenced" versus "named" versus "listed"

referenced...

+ errmsg_plural("cannot attach table \"%s\" as partition because it is
referenced in EXCEPT clause of publication %s",
+ "cannot attach table \"%s\" as partition because it is referenced
in EXCEPT clause of publications %s",

versus (named)

+ errdetail("Partition root \"%s\" is named in the publication's
EXCEPT clause for schema \"%s\".",
+ quote_qualified_identifier(get_namespace_name(get_rel_namespace(root)),
+ get_rel_name(root)),

+ errdetail("Partition root \"%s\" is named in the publication's
EXCEPT clause for schema \"%s\".",
+ RelationGetQualifiedRelationName(pri->relation),
+ get_namespace_name(RelationGetNamespace(pri->relation)))));

versus (listed)

+ errdetail("Ancestor \"%s\" of partition \"%s\" is currently listed
in the EXCEPT clause of the publication.",
+ quote_qualified_identifier(get_namespace_name(get_rel_namespace(root)),
+ get_rel_name(root)),

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

+ * publication, excluding any tables listed in the EXCEPT clause.

+ errdetail("Ancestor \"%s\" of partition \"%s\" is currently listed
in the EXCEPT clause of this command.",
+ RelationGetQualifiedRelationName(pri->relation),
+ quote_qualified_identifier(get_namespace_name(get_rel_namespace(explicitrelid)),

~

IMO there is no real difference here in saying "referenced" v "named"
v "listed", so why not make them all "named" so they are alike.

~~~

1c.
"EXCEPT list" versus "EXCEPT clause"

EXCEPT list:

+ errhint("Use ALTER PUBLICATION ... SET TABLES IN SCHEMA ... EXCEPT
to modify the EXCEPT list.")));

+ * A partition cannot be included when its partition root is in the
+ * same publication's EXCEPT list. Doing so would leave the catalog

+ errhint("Change EXCEPT list using ALTER PUBLICATION ... SET TABLES
IN SCHEMA ... EXCEPT."));

+ errhint("Change EXCEPT list using ALTER PUBLICATION ... SET TABLES
IN SCHEMA ... EXCEPT.")));

+ /* filter out any tables that appear in the EXCEPT list */

+ * A partition whose root is in the publication's EXCEPT list
+ * is also excluded, even if the partition itself lives in a

+ * Validate that each excluded table is not also in the
+ * explicit table list (which would be contradictory), and
+ * that no explicit partition's root is in the except list.

etc.

I don't recognise the reasons for the difference -- it all seems a bit
random. There are a lot more "EXCEPT clause" than there are "EXCEPT
list".

EXCEPT list makes sense for some comments and parameters, but for
user-facing messages maybe those should consistently say "EXCEPT
clause"?

======
src/backend/catalog/pg_publication.c

2.
publication_add_relation:
On Mon, Jul 20, 2026 at 6:22 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Thu, Jul 16, 2026 at 2:04 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
>
> > 7.
> > + errdetail("Ancestor \"%s\" of partition \"%s\" is currently listed
> > in the EXCEPT clause of the publication.",
> > + quote_qualified_identifier(get_namespace_name(get_rel_namespace(root)),
> >
> > Similar (singular) comment to before.
> >
> > /in the EXCEPT clause/in an EXCEPT clause/
> >
>
> It looks like comment #4 is repeated here. In any case, I addressed
> both occurrences of the errdetail (in check_publication_add_relation()
> and CheckExceptNotInTableList()):
> "Ancestor \"%s\" of partition \"%s\" is curre..".
>

Looks like a previous review comment #7 was missed despite saying it
was addressed. It still says:
+ errdetail("The table is currently listed in the EXCEPT clause of the
publication."),

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

ProcessSchemaExceptTables

3.
(patch 0002)

+ * *schemas: accumulates all schema OIDs seen so far in this statement.
+ * *schemas_with_except: tracks which schema OIDs have already been seen
+ * with an EXCEPT clause in this statement.

Maybe this can clarify that `*schemas_with_except` OIDs will be a
subset of `*schemas`.OIDs

SUGGESTION
*schemas_with_except: This is a subset of *schemas. Tracks which of
the seen schemas of this statement had an EXCEPT clause.

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