On Wed, Jun 22, 2022 at 2:18 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jun 16, 2022 at 6:07 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > Thank you for your review comments. Those reported mistakes are fixed > > in the attached patch v3. > > > > This patch looks mostly good to me except for a few minor comments > which are mentioned below. It is not very clear in which branch(es) we > should commit this patch? As per my understanding, this is a > pre-existing behavior but we want to document it because (a) It was > not already documented, and (b) we followed it for row filters in > PG-15 it seems that should be explained. So, we have the following > options (a) commit it only for PG-15, (b) commit for PG-15 and > backpatch the relevant sections, or (c) commit it when branch opens > for PG-16. What do you or others think?
Even though this is a very old docs omission, AFAIK nobody ever raised it as a problem before. It only became more important because of the PG15 row-filters. So I think option (a) is ok. > > Few comments: > ============== > 1. > > > - particular event types. By default, all operation types are replicated. > - (Row filters have no effect for <command>TRUNCATE</command>. See > - <xref linkend="logical-replication-row-filter"/>). > + particular event types. By default, all operation types are replicated. > + These are DML operation limitations only; they do not affect the initial > + data synchronization copy. > > > > Using limitations in the above sentence can be misleading. Can we > change it to something like: "These publication specifications apply > only for DML operations; they do ... ". > OK - modified as suggested. > 2. > + operations. The publication <literal>pub3b</literal> has a row filter. > > In the Examples section, you have used row filter whereas that section > is later in the docs. So, it is better if you give reference to that > section in the above sentence (see Section ...). > OK - added xref as suggested. > 3. > + <para> > + This parameter only affects DML operations. In particular, the > + subscription initial data synchronization does not take > this parameter > + into account when copying existing table data. > + </para> > > In the second sentence: "... subscription initial data synchronization > ..." doesn't sound appropriate. Can we change it to something like: > "In particular, the initial data synchronization (see Section ..) in > logical replication does not take this parameter into account when > copying existing table data."? > OK - modified and added xref as suggested. ~~ PSA patch v4 to address all the above review comments. ------ Kind Regards, Peter Smith. Fujitsu Australia
v4-0001-PGDOCS-tablesync-ignores-publish-operation.patch
Description: Binary data
