Re: Logical replication row filter loses unchanged toasted columns

On Tue, Sep 1, 2026 at 4:55 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Sep 1, 2026 at 12:27 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > I analyzed the issue further and found that the same allocation
> > failure already exists through other code paths, independent of the
> > patch.
> >
> > For both examples, I used the following setup:
> > CREATE TABLE tab_huge (id int PRIMARY KEY, a text, b text, c text);
> > ALTER TABLE tab_huge ALTER COLUMN a SET STORAGE EXTERNAL;
> > ALTER TABLE tab_huge ALTER COLUMN b SET STORAGE EXTERNAL;
> > ALTER TABLE tab_huge ALTER COLUMN c SET STORAGE EXTERNAL;
> > INSERT INTO tab_huge VALUES (1, repeat('a', 400 * 1024 * 1024));
> > -- Add another 400 MB value to the row.
> > UPDATE tab_huge SET b = repeat('b', 400 * 1024 * 1024) WHERE id = 1;
> > -- Add another 400 MB value, bringing the total row data to ~1.2 GB.
> > UPDATE tab_huge SET c = repeat('c', 400 * 1024 * 1024) WHERE id = 1;
> >
> > Example 1: REPLICA IDENTITY FULL
> > ALTER TABLE tab_huge REPLICA IDENTITY FULL;
> > UPDATE tab_huge SET id = 2 WHERE id = 1;
> > ERROR: invalid memory alloc request size 1258291264
> >
> > Example 2: Inserting a row with more than 1 GB of data
> > INSERT INTO tab_huge VALUES (2, repeat('a', 400 * 1024 * 1024),
> > repeat('a', 400 * 1024 * 1024), repeat('a', 400 * 1024 * 1024));
> > ERROR: invalid memory alloc request size 1258291264
> >
> > This suggests that the allocation failure is not necessarily
> > introduced by the patch. The same limitation can already be triggered
> > by existing code paths when PostgreSQL needs to form or flatten a
> > tuple containing more than 1 GB of data. Given this, the failure in
> > the earlier test may be an existing limitation rather than a failure
> > caused by the patch.
> >
>
> Agreed that the allocation of more than 1GB for a single row is our
> limitation besides the bug-fix patch we are discussing as can be seen
> by both INSERT/UPDATE cases. Though UPDATE case is slightly different
> as there we are generating such a large tuple (by fetching the entire
> toast tuple data to WAL log as old_tuple) for the REPLICA IDENTIFY
> FULL case, so one could expect it to work. So, if we want to leave
> this limitation for UPDATE/INSERT as it is and just document it (if
> not documented already) then it is okay to proceed with the current
> approach to fix the issue reported by adding another case to the
> limitation.

+1

>
> Even if we get a pass while generating a tuple here, generating a WAL
> record would result in an ERROR for such a large tuple as there we
> have a limit of XLogRecordMaxSize which is less than 1GB. I can't
> think of an easy fix for this, the two possible ways could be:
>
> 1. Chunk the value directly into WAL via a new, dedicated record type,
> whose redo is a no-op on the physical database; just bytes riding in
> WAL for logical decoding to consume. This avoids
> heap_insert()/index_insert() overhead entirely per chunk, so it's the
> more efficient option.
> 2. Reuse the existing toast-chunk machinery: below a safety margin
> under XLogRecordMaxSize, behavior is unchanged. Above it, persist each
> out-of-line value by writing it out as ordinary toast chunk rows under
> a fresh toast id (streaming from the existing chunks, never
> materializing the whole value in memory), then deleting those rows
> again within the same transaction, so nothing is left live for VACUUM.
> The old tuple keeps only a small placeholder pointer, resolved at
> decode time the same way ReorderBufferToastReplace() already resolves
> changed values for the new tuple — generalized to also handle the old
> tuple.
>
> I went with (2) as the proof of concept as the (1) requires new WAL
> record type and new decoding/reorder buffer logic to reassemble this
> new WAL record_type. This POC patch has been written with the help of
> AI. If this approach is acceptable, we'd extend Hou-san's row-filter
> patch to reuse the same building blocks (the size-check and the
> WAL-only-chunk helper) for the row-filter case, so an oversized
> unchanged column there degrades the same way instead of risking the
> same failure.
>
> I don't think this is a good candidate for backpatching irrespective
> of whichever approach we choose ((1) or (2)) to fix this issue.

Given that this is a HEAD-only patch it might be better to discuss in
a separate thread, but let me share my thoughts on these ideas.

I'm concerned about side effects by (2): IIUC it inserts toast chunks
and deletes immediately within the same transaction in order just to
convey these toast chunks into WAL stream. It would make the shared
buffer dirty, possibly consume disk I/O, and require vacuum to
physically remove them. Because the patch targets heap tuples larger
than 1GB, there would be a huge impact on the database. While (1)
introduces a new WAL record type, it sounds better to me. I've not
considered other potential ideas though.

BTW the commit message of the patch says:

REPLICA IDENTITY FULL unconditionally inlines every out-of-line column
value into the old tuple via toast_flatten_tuple() before WAL-logging
it. When the combined size is large enough, this can fail outright
with "invalid memory alloc request size", or, in a narrower window,
succeed but produce a WAL record exceeding XLogRecordMaxSize. That
failure is only discovered inside XLogInsert(), after heap_update()/
heap_delete() have already entered the critical section, turning what
should be an ordinary ERROR into a PANIC.

In which case does heap_update() or heap_delete() on a RI FULL table
succeed but produce a WAL record exceeding XLogRecordMaxSize? I think
we should fix it in any case as it leads to a server crash.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com