Re: Support EXCEPT for TABLES IN SCHEMA publications

Hi Nisha.

Some review comments for the v12 patches.

//////
v12-0001

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

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

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

RemovePublicationExceptForRelation:

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

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

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

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

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