On Tue, Jan 13, 2026 at 6:15 PM Dilip Kumar <[email protected]> wrote: > > On Tue, Jan 13, 2026 at 5:02 PM shveta malik <[email protected]> wrote: > > > > On Tue, Jan 13, 2026 at 4:09 PM Dilip Kumar <[email protected]> wrote: > > > > > > On Tue, Jan 13, 2026 at 3:59 PM shveta malik <[email protected]> > > > wrote: > > > > > > > > On Sat, Jan 10, 2026 at 6:45 PM Dilip Kumar <[email protected]> > > > > 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. > > To clarify, I’m allowing INSERT and UPDATE alongside DELETE not > because I anticipate a specific use case, but to avoid adding > unnecessary code complexity. Since restricting these operations isn't > a safety requirement, I felt it was better to keep the implementation > simple rather than adding extra checks that don't provide real value. >
What kind of complexity you are anticipating, can you show by top-up patch? I think allowing just DELETE and TRUNCATE to table owners would be ideal. > So let's get consensus on this decision and then we can change accordingly. > > > > > > > >> 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. > > Thanks for your opinion. > > > > > > > > > > > > > 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. > > That’s an interesting point. I initially considered creating internal > indexes to simplify management and avoid requiring users to have DDL > permissions on the pg_conflict schema. However, I realized that > predefined indexes might be too restrictive for diverse search > requirements. Perhaps we should omit indexes from the initial > version? We can then decide whether to add predefined indexes or allow > external ones based on actual usage patterns and feedback. Thoughts? > I agree that the indexes could be considered later unless required by internal code to search the conflict table. -- With Regards, Amit Kapila.
