Re: Logical replication row filter loses unchanged toasted columns

On Thu, Aug 20, 2026 at 1:02 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Wed, Aug 19, 2026 at 10:55 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Aug 20, 2026 at 4:54 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > 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().
>
> I think we can somehow check (a) during heap_update(). But I agree
> that we should not check (b) during heap_update().
>

Fair enough. I think we can try to come up with a patch and see if it
is good for both HEAD and back-branches. BTW, one thing to note in at
least the current posted patch, there is an ABI break with new member
rf_exists_for_update in PublicationDesc. I feel as it can be added at
the end of structure, the chances of breaking any existing extension
are very less. Also, the new parameter in
pub_rf_contains_invalid_column could be harmless as the function
shouldn't be used but we can overcome that by having a separate
function like pub_has_row_filter() as shown in Hou-San's patch for
PG18 version.

> >
> > 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.
>
> While the proposed fix is less risky, I'd like to be clear about what
> a user can actually do when it fires. The patch suggests "To enable
> updating the table, set REPLICA IDENTITY FULL or remove the row filter
> from the publication.". As far as I can see, possible actions are:
>
> 1. Set RI FULL.
> 2. Remove the row filter from the publication.
> 3. Rewrite the offending statements to also modify the out-of-line column.
> 4. Change the application so that it never issues such updates.
>
> (1) doesn't actually work if the publication has a column list.
> pub_contains_invalid_column() rejects that combination (RI FULL +
> column list). So every UPDATE on the table fails. Even if users remove
> the column list from the publication, RI FULL logs the whole old tuple
> on every UPDATE, which is more WAL and more CPU than what I proposed.
>
> (2) could require some system architecture changes. Dropping the
> filter sends the subscriber rows it was deliberately not supposed to
> receive, and adds the network traffic and apply cost for all of them.
>
> (3) works: e.g., "UPDATE t SET id = 7, body = body || ''" passes the
> check. But it writes a fresh copy of the value into the toast table,
> so it costs at least as much WAL as logging the old value would have,
> and leaves the previous toast rows behind for vacuum.
>
> (4) I don't think this is reachable in practice.
>
> Am I missing an option? If there is a workaround that keeps both the
> row filter and the column list, and does not cost more than the WAL we
> are trying to avoid, it would be great. Otherwise, I'm concerned that
> prohibiting these updates leaves users without a practical answer.
>

If we have to give an ERROR during heap_update then we won't have an
alternative but if we follow what Nikhil has proposed there is another
alternative to use Alter Subscription ... Skip on subscriber-side but
not sure if it is a good idea.

--
With Regards,
Amit Kapila.