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

On Tue, Jan 13, 2026 at 4:09 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> 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 don’t see a strong reason for a user to manually perform INSERT or
UPDATE here. But on rethinking, I also agree that allowing it does no
harm. It simply gives the user flexibility to modify data they “own”,
i.e., data that the system does not internally reference. Personally,
I’m okay with leaving it unrestricted, but it will be good to document
it as a NOTE/CAUTION, stating that these DML operations are allowed
and it is the user’s responsibility to manage any changes they make
manually.

>
> >> 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?
>

I am okay if we allow DELETE alone.

> >
> >
> > 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.

Right. Do you think we shall allow index creation as well on this
table? It may help if the table is huge. As an example, a user can
create an index on relid to query it faster. Currently it is not
allowed.

> > 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.

Okay.

thanks
Shveta