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: