Re: Column Filtering in Logical Replication - Mailing list pgsql-hackers

On Mon, Aug 22, 2022 at 1:58 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Thanks for the view of v1-0001.
>
> On Wed, Aug 17, 2022 at 3:04 AM vignesh C <vignesh21@gmail.com> wrote:
> ...
> > 1) Row filters mentions that "It has no effect on TRUNCATE commands.",
> > the same is not present in case of column filters. We should keep the
> > changes similarly for consistency.
> > --- a/doc/src/sgml/ref/create_publication.sgml
> > +++ b/doc/src/sgml/ref/create_publication.sgml
> > @@ -90,8 +90,7 @@ CREATE PUBLICATION <replaceable
> > class="parameter">name</replaceable>
> >       <para>
> >        When a column list is specified, only the named columns are replicated.
> >        If no column list is specified, all columns of the table are replicated
> > -      through this publication, including any columns added later.  If a column
> > -      list is specified, it must include the replica identity columns.
> > +      through this publication, including any columns added later.
>
> Modified as suggested.
>
> >
> > 2) The document says that "if the table uses REPLICA IDENTITY FULL,
> > specifying a column list is not allowed.":
> > +   publishes only <command>INSERT</command> operations. Furthermore, if the
> > +   table uses <literal>REPLICA IDENTITY FULL</literal>, specifying a column
> > +   list is not allowed.
> > +  </para>
> >
> > Did you mean specifying a column list during create publication for
> > REPLICA IDENTITY FULL table like below scenario:
> > postgres=# create table t2(c1 int, c2 int, c3 int);
> > CREATE TABLE
> > postgres=# alter table t2 replica identity full ;
> > ALTER TABLE
> > postgres=# create publication pub1 for table t2(c1,c2);
> > CREATE PUBLICATION
> >
> > If so, the document says specifying column list is not allowed, but
> > creating a publication with column list on replica identity full was
> > successful.
>
> That patch v1-0001 was using the same wording from the github commit
> message [1]. I agree it was a bit vague.
>
> In fact the replica identity validation is done at DML execution time
> so your example will fail as expected when you attempt to do a UPDATE
> operation.
>
> e.g.
> test_pub=# update t2 set c2=23 where c1=1;
> ERROR:  cannot update table "t2"
> DETAIL:  Column list used by the publication does not cover the
> replica identity.
>
> I modified the wording for this part of the docs.

Few comments:
1) I felt no expressions are allowed in case of column filters. Only
column names can be specified. The second part of the sentence
confuses what is allowed and what is not allowed. Won't it be better
to remove the second sentence and mention that only column names can
be specified.
+   <para>
+    Column list can contain only simple column references. Complex
+    expressions, function calls etc. are not allowed.
+   </para>

2) tablename should be table name.
+   <para>
+    A column list is specified per table following the tablename, and
enclosed by
+    parenthesis. See <xref linkend="sql-createpublication"/> for details.
+   </para>

We have used table name in the same page in other instances like:
a) The row filter is defined per table. Use a WHERE clause after the
table name for each published table that requires data to be filtered
out. The WHERE clause must be enclosed by parentheses.
b) The tables are matched between the publisher and the subscriber
using the fully qualified table name.

3) One small whitespace issue:
git am v2-0001-Column-List-replica-identity-rules.patch
Applying: Column List replica identity rules.
.git/rebase-apply/patch:30: trailing whitespace.
   if the publication publishes only <command>INSERT</command> operations.
warning: 1 line adds whitespace errors.

Regards,
Vignesh