Re: Proposal: Conflict log history table for Logical Replication

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: 2026-09-26 01:04:02
Message-ID: CAA4eK1+qEDF6QZW0qseLC84v+aQGoGXXUAAicrcxfpEzznhPnQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 22, 2026 at 10:56 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> A solution approach for both of these issues:
> ---------------------------------------------------------
> Case-B: when a column's type has a user-defined cast to json,
> row_to_json() uses that cast instead of the type's normal output
> function. The cast can return anything of any size no matter how small
> the stored value is, so a 4-byte key can produce more than 1GB of json
> and break apply. It applies inside arrays and composites too, because
> row_to_json() walks into each element.
>
> The LOG case never had this problem, because it prints each key column
> with the type's output function and a cast cannot replace that. So we
> do the same: render each value with its output function and build the
> json object ourselves. No cast is then reachable at any nesting depth.
>
> The cost is that values become json strings instead of numbers,
> booleans or nested objects. With a int PRIMARY KEY, {"a":1} becomes
> {"a":"1"}, and a jsonb key becomes a quoted string.
> This does not reduce what can be queried. "->>" still returns a value
> that can be cast back to the column’s type, so
> (replica_identity->>'a')::int can retrieve the key and be used to find
> the row. Only querying inside a container key requires one extra cast,
> e.g. (replica_identity->>'doc')::jsonb->>'x'.
>
> Case A: With case B handled, adding a per-column size cap before
> rendering becomes straightforward. 1kB looks like the right size.
> Roughly, the rendered value per key column is
> escape_json(typoutput(value)) plus small overhead, and:
> - escape_json expands at most 6x, since every byte below 0x20 becomes \u00XX
> - INDEX_MAX_KEYS bounds the column count at 32
> - the cap bounds each varlena input at 1kB
>
> Among built-in types only numeric produces output vastly larger than
> its storage: length(1e131071::numeric::text) is 131,072, so 10 bytes
> becomes 131kB. Repeating that inside a container (array, jsonb or
> multirange) gives at most about 11,000x per stored byte. See the case
> at [1].
>
> So the worst case at 1kB is 1kB * 11,000 * 32 = ~360MB, a 3x margin
> below the 1GB limit. 2kB gives ~720MB, only 1.5x, which seems thin.
> 4kB exceeds 1GB outright. Hence 1kB.
>
> Fixed-length columns are not size-checked, as their output is bounded
> by construction and adds only a few MB across 32 columns.
>

The solution for these problems in the attached patch looks reasonable to me.

> What remains:
> Together these cover every built-in type, scalar and container alike.
> The one case neither handles is a user-defined type whose own output
> function renders far more than its input; no cap on the input can
> detect that. Such a function has to be written in C, since a SQL
> function cannot return cstring, so it sits at the same level as
> replacing a built-in output function. IMO, that seems acceptable to
> leave for now.
>

I have tried to verify this with the attached extension (written with
the help of AI). Here, I want to be precise with one thing that with
such a misbehaved out function, even by default replication will fail
because we use text format to transfer data and the same out function
will be invoked leading to an allocation failure. In binary format, it
will pass. So, in the attached patch, if remove "binary = true" from
the following statement:
+ CONNECTION '$publisher_connstr application_name=$appname'
+ PUBLICATION pub_bt WITH (binary = true, conflict_log_destination = all)"

the test will fail with publisher LOG printing:

2026-09-26 05:54:10.888 IST walsender[26238] sub_bt ERROR: string
buffer exceeds maximum allowed length (1073741823 bytes)
2026-09-26 05:54:10.888 IST walsender[26238] sub_bt DETAIL: Cannot
enlarge string buffer containing 600000049 bytes by 600000000 more
bytes.
2026-09-26 05:54:10.888 IST walsender[26238] sub_bt CONTEXT: slot
"sub_bt", output plugin "pgoutput", in the change callback, associated
LSN 0/018043D0

So, it is fine to leave this as is. Also, as such a function has to be
written in C which means one can write something to even crash the
backend (can read/write arbitrary memory) which is way worse than
allocation ERROR.

Sawada-San, what do you think about the above cases?

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2026-09-26 01:06:02 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Bharath Rupireddy 2026-09-25 23:25:18 Re: WAL segment file descriptor leak on read errors can PANIC the server