Re: row filtering for logical replication

On Fri, Jan 21, 2022 at 2:04 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Jan 20, 2022 at 7:56 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >
> > > > Maybe this was meant to be "validate RF
> > > > expressions" and return, perhaps, a bitmapset of all invalid columns
> > > > referenced?
> > >
> > > Currently, we stop as soon as we find the first invalid column.
> >
> > That seems quite strange. (And above you say "gather as much info as
> > possible", so why stop at the first one?)
> >
>
> Because that is an error case, so, there doesn't seem to be any
> benefit in proceeding further. However, we can build all the required
> information by processing all publications (aka gather all
> information) and then later give an error if that idea appeals to you
> more.
>
> > > > (What is an invalid column in the first place?)
> > >
> > > A column that is referenced in the row filter but is not part of
> > > Replica Identity.
> >
> > I do wonder how do these invalid columns reach the table definition in
> > the first place. Shouldn't these be detected at DDL time and prohibited
> > from getting into the definition?
> >
>
> As mentioned by Peter E [1], there are two ways to deal with this: (a)
> The current approach is that the user can set the replica identity
> freely, and we decide later based on that what we can replicate (e.g.,
> no updates). If we follow the same approach for this patch, we don't
> restrict what columns are part of the row filter, but we check what
> actions we can replicate based on the row filter. This is what is
> currently followed in the patch. (b) Add restrictions during DDL which
> is not as straightforward as it looks.

FYI - I also wanted to highlight that doing the replica identity
validation at update/delete time is not only following the "current
approach", as mentioned above, but this is also consistent with the
*documented* behaviour in PG docs (See [1] since PG v10),

<QUOTE>
If a table without a replica identity is added to a publication that
replicates UPDATE or DELETE operations then subsequent UPDATE or
DELETE operations will cause an error on the publisher.
</QUOTE>

Specifically,

It does *not* say that the RI validation error will happen when a
table is added to the publication at CREATE/ALTER PUBLICATION time.

It says that *subsequent* "UPDATE or DELETE operations will cause an error".

~~

The point is that it is one thing to decide to change something that
was never officially documented, but to change already *documented*
behaviour is much more radical and has the potential to upset some
users.

------
[1] https://www.postgresql.org/docs/devel/logical-replication-publication.

Kind Regards,
Peter Smith.
Fujitsu Australia