Re: Skipping schema changes in publication - Mailing list pgsql-hackers

On Thu, 30 Oct 2025 at 11:34, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Vignesh
>
> Here are some review comments for the patch v24-0002.
>
> These comments are just for the SGML docs. The patch needs a rebase so
> I was unable to review the code.
>
> ======
> Commit message
>
> 1.
> A new column "prexcept" is added to table "pg_publication_rel", to maintain
> the relations that the user wants to exclude from the publications.
>
> ~
>
> /to maintain/to flag/
>
> ======
> doc/src/sgml/logical-replication.sgml
>
> 2.
>    <para>
> -   To add tables to a publication, the user must have ownership rights on the
> -   table. To add all tables in schema to a publication, the user must be a
> -   superuser. To create a publication that publishes all tables or
> all tables in
> -   schema automatically, the user must be a superuser.
> +   To create a publication using FOR ALL TABLES or FOR ALL TABLES IN SCHEMA,
> +   the user must be a superuser. To add ALL TABLES or ALL TABLES IN SCHEMA to a
> +   publication, the user must be a superuser. To add tables to a publication,
> +   the user must have ownership rights on the table.
>    </para>
>
> Those "FOR ALL TABLES" etc are missing SGML markup.
>
> ======
> doc/src/sgml/ref/alter_publication.sgml
>
> 3.
> +ALTER PUBLICATION <replaceable class="parameter">name</replaceable>
> ADD ALL TABLES [ EXCEPT [ TABLE ] <replaceable
> class="parameter">exception_object</replaceable> [, ... ] ]
>
> and
>
> +
> +<phrase>where <replaceable
> class="parameter">exception_object</replaceable> is:</phrase>
> +
> +    [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
> +
>
> It is not clear from the syntax which of these is possible.
>
> ... ADD ALL TABLES EXCEPT TABLE t1,t2,t3
> ... ADD ALL TABLES EXCEPT TABLE t1, TABLE t2, TABLES t3
>
> IMO it is best put the "[TABLE]" within the exception_object:
> [ TABLE ] [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
>
> Then both are possible, which is consistent with how "FOR TABLE" syntax works.
>
> Furthermore, you might want later to say EXCLUDE SEQUENCE, so doing it
> this way makes that possible.
>
> ~~~
>
> 4.
> -   Adding a table to a publication additionally requires owning that table.
> -   The <literal>ADD TABLES IN SCHEMA</literal>,
> +   Adding a table to or excluding a table from a publication additionally
> +   requires owning that table. The <literal>ADD ALL TABLES</literal>,
>
> This wording seems a bit awkward. How are re-phrasing like:
>
> SUGGESTION
> Adding or excluding a table from a publication requires ownership of that table.
>
> ~~~
>
> 5.
> -      name to explicitly indicate that descendant tables are included.
> +      name to explicitly indicate that descendant tables are affected. For
> +      partitioned tables, <literal>ONLY</literal> donot have any effect.
>
> typo: /donot/does not/
>
> ======
> doc/src/sgml/ref/create_publication.sgml
>
> 6.
> -    [ FOR ALL TABLES
> +    [ FOR ALL TABLES [ EXCEPT [ TABLE ] <replaceable
> class="parameter">exception_object</replaceable> [, ... ] ]
>        | FOR <replaceable
> class="parameter">publication_object</replaceable> [, ... ] ]
>      [ WITH ( <replaceable
> class="parameter">publication_parameter</replaceable> [= <replaceable
> class="parameter">value</replaceable>] [, ... ] ) ]
>
> @@ -30,6 +30,10 @@ CREATE PUBLICATION <replaceable
> class="parameter">name</replaceable>
>
>      TABLE [ ONLY ] <replaceable
> class="parameter">table_name</replaceable> [ * ] [ ( <replaceable
> class="parameter">column_name</replaceable> [, ... ] ) ] [ WHERE (
> <replaceable class="parameter">expression</replaceable> ) ] [, ... ]
>      TABLES IN SCHEMA { <replaceable
> class="parameter">schema_name</replaceable> | CURRENT_SCHEMA } [, ...
> ]
> +
> +<phrase>where <replaceable
> class="parameter">exception_object</replaceable> is:</phrase>
> +
> +    [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
>
> Same review comment as #3 before.
>
> I think it is clearer (and more flexible) to change the
> exception_object to include [TABLE].
> [ TABLE ] [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ]
>
> It also helps pave the way for any future EXCLUDE SEQUENCE feature.
>
> ~~~
>
> 7.
> +     <para>
> +      This clause specifies a list of tables to be excluded from the
> +      publication. It can only be used with <literal>FOR ALL TABLES</literal>.
> +      If <literal>ONLY</literal> is specified before the table name, only
> +      that table is excluded from the publication. If
> <literal>ONLY</literal> is
> +      not specified, the table and all its descendant tables (if any) are
> +      excluded. Optionally, <literal>*</literal> can be specified after the
> +      table name to explicitly indicate that descendant tables are excluded.
> +      This does not apply to a partitioned table, however.  The partitioned
> +      table or its partitions are excluded from the publication based on the
> +      parameter <literal>publish_via_partition_root</literal>.
> +     </para>
> +     <para>
> +      When <literal>publish_via_partition_root</literal> is set to
> +      <literal>true</literal>, specifying a root partitioned table in
> +      <literal>EXCEPT TABLE</literal> excludes it and all its partitions from
> +      replication. Specifying a leaf partition has no effect, as its
> changes are
> +      still replicated via the root partitioned table. When
> +      <literal>publish_via_partition_root</literal> is set to
> +      <literal>false</literal>, specifying a partitioned table or non-leaf
> +      partition has no effect, as changes are replicated via the leaf
> +      partitions. Specifying a leaf partition excludes only that partition from
> +      replication.
> +     </para>
>
> I felt that the second paragraph should be started with the sentence
> "The partitioned table or its partitions are excluded...", so then
> everything related to "publish_via_partition_root" is kept together.
>
> ~~~
>
> 8.
> +  <para>
> +   Create a publication that publishes all changes in all the tables except for
> +   the changes of <structname>users</structname> and
> +   <structname>departments</structname>:
> +<programlisting>
> +CREATE PUBLICATION mypublication FOR ALL TABLES EXCEPT users, departments;
> +</programlisting>
> +  </para>
>
> The words "the changes of" are not needed, and you did not use that
> wording in the ALTER PUBLICATION example.
>
> ======
> doc/src/sgml/ref/psql-ref.sgml
>
> 9.
>          If <literal>x</literal> is appended to the command name, the results
>          are displayed in expanded mode.
> -        If <literal>+</literal> is appended to the command name, the tables and
> -        schemas associated with each publication are shown as well.
> +        If <literal>+</literal> is appended to the command name, the tables,
> +        excluded tables and schemas associated with each publication
> are shown as
> +        well.
>          </para>
>
> /excluded tables and schemas/excluded tables, and schemas/
>
Hi Peter, Vignesh

Thanks for reviewing the patches.
I have rebased the patches. I have modified the syntax for EXCEPT
TABLE (002) patch.
For example, now to exclude a table we need to specify like:
CREATE PUBLICATION pub1 FOR ALL TABLE EXCEPT TABLE (t1, t2);
We need to specify '()' around the table list.

This patchset is the only rebased version. I will address all the
comments in the next version of patch.

Thanks,
Shlok Kyal