On Wed, Sep 2, 2026 at 6:05 AM Masahiko Sawada <[email protected]> wrote:
>
> On Tue, Sep 1, 2026 at 4:55 AM Amit Kapila <[email protected]> wrote:
> >
> > 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
>
So, we can proceed with the current direction but with additional
documentation on the limitations.
> >
> > 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.
>
Agreed, this is a change large enough that we need a separate thread
to discuss and develop a patch for it.
> 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.
>
Agreed, I would also prefer (1) among these unless there is any other
better way to achieve this.
> 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.
>
As the MaxAllocSize is about 4MB larger than XLogRecordMaxSize, so the
record_size has to fit in-between those two. The following test
reproduces the problem for me:
CREATE TABLE t (id int PRIMARY KEY, a text);
ALTER TABLE t ALTER COLUMN a SET STORAGE EXTERNAL;
ALTER TABLE t REPLICA IDENTITY FULL;
INSERT INTO t VALUES (1, repeat('a', 1021*1024*1024));
DELETE FROM t WHERE id = 1;
The idea proposed to handle large sized toast data for the RI FULL
case should address this problem as well. I don't think we should try
to address this problem along with the row_filter related issue for
which we agreed to proceed with Hou-San's latest patch version idea.
--
With Regards,
Amit Kapila.
