Re: row filtering for logical replication

On Sat, Dec 4, 2021 at 4:43 AM Euler Taveira <euler(at)eulerto(dot)com> wrote:
>
> On Thu, Dec 2, 2021, at 4:18 AM, Peter Smith wrote:
>
> PSA a new v44* patch set.
>
> We are actively developing this feature for some months and we improved this
> feature a lot. This has been a good team work. It seems a good time to provide
> a retrospective for this feature based on the consensus we reached until now.
>
> The current design has one row filter per publication-table mapping. It allows
> flexible choices while using the same table for multiple replication purposes.
> The WHERE clause was chosen as the syntax to declare the row filter expression
> (enclosed by parentheses).
>
> There was a lot of discussion about which columns are allowed to use in the row
> filter expression. The consensus was that publications that publish UPDATE
> and/or DELETE operations, should check if the columns in the row filter
> expression is part of the replica identity. Otherwise, these DML operations
> couldn't be replicated.
>
> We also discussed about which expression would be allowed. We couldn't allow
> all kind of expressions because the way logical decoding infrastructure was
> designed, some expressions could break the replication. Hence, we decided to
> allow only "simple expressions". By "simple expression", we mean to restrict
> (a) user-defined objects (functions, operators, types) and (b) immutable
> builtin functions.
>

I think what you said as (b) is wrong because we want to allow builtin
immutable functions. See discussion [1].

> A subscription can subscribe to multiple publications. These publication can
> publish the same table. In this case, we have to combine the row filter
> expression to decide if the row will be replicated or not. The consensus was to
> replicate a row if any of the row filters returns true. It means that if one
> publication-table mapping does not have a row filter, the row will be
> replicated. There is an optimization for this case that provides an empty
> expression for this table. Hence, it bails out and replicate the row without
> running the row filter code.
>

In addition to this, we have decided to have an exception/optimization
where we need to consider publish actions while combining multiple
filters as we can't combine insert/update filters.

> The same logic applies to the initial table synchronization if there are
> multiple row filters. Copy all rows that satisfies at least one row filter
> expression. If the subscriber is a pre-15 version, data synchronization won't
> use row filters if they are defined in the publisher.
>
> If we are dealing with partitioned tables, the publication parameter
> publish_via_partition_root determines if it uses the partition row filter
> (false) or the root partitioned table row filter (true).
>
> I used the last patch series (v44) posted by Peter Smith [1]. I did a lot of
> improvements in this new version (v45). I merged 0001 (it is basically the main
> patch I wrote) and 0004 (autocomplete). As I explained in [2], I implemented a
> patch (that is incorporated in the v45-0001) to fix this issue. I saw that
> Peter already proposed a slightly different patch (0006). I read this patch and
> concludes that it would be better to keep the version I have. It fixes a few
> things and also includes more comments. I attached another patch (v45-0002)
> that includes the expression validation. It is based on 0002. I completely
> overhaul it. There are additional expressions that was not supported by the
> previous version (such as conditional expressions [CASE, COALESCE, NULLIF,
> GREATEST, LEAST], array operators, XML operators). I probably didn't finish the
> supported node list (there are a few primitive nodes that need to be checked).
> However, the current "simple expression" routine seems promising. I plan to
> integrate v45-0002 in the next patch version. I attached it here for comparison
> purposes only.
>
> My next step is to review 0003. As I said before it would like to treat it as a
> separate feature.
>

I don't think that would be right decision as we already had discussed
that in detail and reach to the current conclusion based on which
Ajin's 0003 patch is.

> I know that it is useful for data consistency but this patch
> is already too complex.
>

True, but that is the main reason the review and development are being
done as separate sub-features. I suggest still keeping the similar
separation till some of the reviews of each of the patches are done,
otherwise, we need to rethink how to divide for easier review. We need
to retain the 0005 patch because that handles many problems without
which the main patch is incomplete and buggy w.r.t replica identity.

[1] - https://www.postgresql.org/message-id/CAA4eK1%2BXoD49bz5-2TtiD0ugq4PHSRX2D1sLPR_X4LNtdMc4OQ%40mail.gmail.com

--
With Regards,
Amit Kapila.