Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

From Dilip Kumar
Subject Re: Proposal: Conflict log history table for Logical Replication
Date
Msg-id CAFiTN-uL9f0X+=Ep4BbAPvaTJA7S4XHM--G4BsnPJw4uJW7EGQ@mail.gmail.com
Whole thread Raw
In response to Re: Proposal: Conflict log history table for Logical Replication  (shveta malik <shveta.malik@gmail.com>)
Responses Re: Proposal: Conflict log history table for Logical Replication
List pgsql-hackers
On Tue, Jan 13, 2026 at 3:59 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Sat, Jan 10, 2026 at 6:45 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> >>
> > Here is the updated patch
> >
>
> Thanks, I will review the code. Few concerns while experimenting with 001 alone:
>
> 1)
> I am able to insert and update rows in pg_conflict.pg_conflict_16394.
> It should be restricted.
>
> 2)
> I think we need to block 'SELECT for UPDATE' and similar operations on
> CLT. Currently it is allowed on CLT.
> See this:
>
> postgres=# SELECT * FROM  pg_toast.pg_toast_3466 for UPDATE;
> ERROR:  cannot lock rows in TOAST relation "pg_toast_3466"
> postgres=# SELECT * FROM pg_conflict.pg_conflict_16394 for UPDATE;
> ....

I sent an analysis on this a few days ago but realized it only went to
Amit. Resending to the full list:

So the summary is, currently, all INSERT/UPDATE/DELETE operations are
blocked for TOAST tables for safety. However, I have allowed these
operations for the pg-conflict table. Unlike TOAST, the system does
not internally reference conflict data, so user manipulation doesn't
pose a safety risk to the system. If a user modifies or deletes this
data, they simply lose their logs without impacting system stability.
What are your thoughts on this approach?


>> I  Wrote
> I have been exploring the enforcement of these restrictions.
> Currently, DML is validated via CheckValidResultRel(). If we do not
> explicitly check for the conflict log table in that function, DML
> operations, including DELETE, will be permitted. While I am not overly
> concerned about users attempting manual INSERT or UPDATE operations.

> Now for TRUNCATE, the truncate_check_rel() currently relies on
> IsSystemClass() to restrict truncations. Since the conflict namespace
> is included in IsSystemClass() to prevent unauthorized DDL, TRUNCATE
> is blocked by default. We could allow it by adding an exception in
> truncate_check_rel() (e.g., IsSystemClass() &&
> !IsConflictNamespace()), but I am unsure if this is necessary. Do we
> really need to permit TRUNCATE, or is allowing DELETE sufficient for
> user-driven cleanup?

>
>
> 3)
> We currently skip ANALYZE on TOAST tables, but I’m not sure whether
> the same restriction should apply to CLT. Since users are expected to
> query CLT frequently, collecting statistics could be beneficial. Even
> though there are currently no indexes or other structures to enable
> more optimal plans, having statistics should not harm. Thoughts?
>
> postgres=# analyze pg_toast.pg_toast_16399;
> WARNING:  skipping "pg_toast_16399" --- cannot analyze non-tables or
> special system tables
>
> postgres=# analyze pg_conflict.pg_conflict_16394;
> ANALYZE

I think its good to ANALYZE CLT data because user logically never need
to query the toast data, but they would have to query the conflict
data.

> 4)
> It will be good to show 'Conflict Log Table:' in \dRs command.

Yeah, you raised this point earlier as well, somehow I am missing this
point from the last few versions. I will analyze this and change this
as a first comment in the next version.

--
Regards,
Dilip Kumar
Google



pgsql-hackers by date:

Previous
From: shveta malik
Date:
Subject: Re: Proposal: Conflict log history table for Logical Replication
Next
From: Alexander Pyhalov
Date:
Subject: Re: Limit memory usage by postgres_fdw batches