Thread
-
Re: Skipping schema changes in publication
Shlok Kyal <shlok.kyal.oss@gmail.com> — 2025-11-06T11:16:48Z
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