Re: Logical replication row filter loses unchanged toasted columns

On Thu, Aug 20, 2026 at 4:54 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Fri, Aug 14, 2026 at 4:50 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > I think even if we want to block operations that can create such a
> > situation, we should reject only the specific updates that lead to the
> > problem, not every update on a table that merely has the potential for
> > it. We already do something similar: UPDATE/DELETE is rejected when
> > there's no replica identity and the table's publications publish those
> > operations. I'd like to apply the same principle here.
> >
> > With that in mind, I could think of following two options:
> >
> > Option 1
> > Check at DML time, inside heap_update(): Detect the problem per-row,
> > at the point where old/new tuple data is actually available when
> > following conditions are met: the relation is published and has
> > UPDATEs enabled, (b) some publication defines a row filter on it, (c)
> > the replica identity key changed value in this UPDATE, (d) the old
> > tuple has some externally-stored (TOASTed) attribute
> > (HeapTupleHasExternal()), and (e) some specific non-replica-identity
> > column's value is unchanged and still stored out-of-line.
> >
> > Only an UPDATE that actually satisfies all five conditions is
> > rejected, with an error naming the offending column. All other UPDATEs
> > on the same table proceed normally, including ones that don't touch
> > the key, or ones where the TOASTed column did change.
>
> It looks like this bug can happen when all of the above five
> conditions are met, which seems to be narrow in practice. If the
> affected cases are that narrow, why don't we log the whole old tuple
> image in exactly that case, instead of erroring out?
>
It can cost us high in some cases that can occur in the narrow window
of above conditions. I could think of two major points to consider:
(a) Changed toasted columns don't need their old value logged at all —
once a column is toasted in this transaction, its new value is already
fully resolvable from the new tuple, so flattening the old tuple just
pays a detoast fetch and extra WAL for a value nothing downstream
reads; (b) The trigger condition (rf_exists_for_update &&
id_key_changed && HeapTupleHasExternal(oldtup)) can't tell whether the
filter's match result actually differs between old and new row (we
need additional WAL only when row_filter matches new row but not old),
since that needs evaluating the filter itself, already rejected as too
invasive inside heap_update().

So, combining both cases, I am worried it could be additional CPU and
WAL cost when the same is not required though only in narrow cases.
But I see there is an argument that all of this work is done when
toasting for both old and new tuples is there which is not a very hot
code path but still I am not sure this additional cost is worth it.

> While it does
> write more WAL, but only in that narrow case, It would be better than
> requiring users to change RI setting or publication settings. Also, I
> guess it can be back-patched as it neither adds a new WAL record type
> nor changes the existing WAL format. Ideally, it would be sufficient
> to write RI + unchanged out-of-line column data. But that needs new
> logic to select the attribute, so I would leave it for the master.
>

Right, we can have this solution for master but thought first we can
have a somewhat less risky fix for backbranches (including master).
Then as a separate task try to improve the situation in master by
considering various options like the one you suggested, or have
something like INCLUDE kind of syntax for RI, or yet another one is to
consider evaluating row_filter for such narrow cases.

--
With Regards,
Amit Kapila.