| From: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(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 09:14:12 |
| Message-ID: | CAFiTN-vi7rcF2CzGjPf3g+yEmYkTwCYRp81kyjEs-tt7ZF08Mw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Sep 26, 2026 at 6:34 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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.
Yeah that analogy makes sense to me.
> Sawada-San, what do you think about the above cases?
Let's hear from Sawada-San.
--
Regards,
Dilip Kumar
Google
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Dirkjan Bussink | 2026-09-26 07:23:42 | Re: Server crash when describing a FETCH statement after its cursor is closed |