On Mon, Jul 6, 2026 at 5:10 AM Peter Smith <[email protected]> wrote: > > On Sat, Jul 4, 2026 at 11:07 PM Dilip Kumar <[email protected]> wrote: > > > > On Fri, Jul 3, 2026 at 11:03 AM Peter Smith <[email protected]> wrote: > > > > > > The wording of the comments above is extremely repetitive. That may > > > not be noteworthy if the comment was just in one place, but it's > > > everywhere. > > > > > > How about like: > > > > > > BEFORE > > > Conflict log tables are used internally for logical replication > > > conflict logging and should not have <xxx>, as it could disrupt > > > conflict logging. > > > > > > SUGGESTION > > > Conflict log tables are internal to logical replication and must not > > > have <xxx>, since it could interfere with their operation. > > > > > > > I agree with most of Amit's comments. Regarding this one, I really > > don't think the suggested comment adds much value over what we > > currently have. However, if anyone feels this must be changed, please > > let me know, I will provide a patch. > > > > Of course, changing or not changing is your call, but just so you > know, whenever I post suggestions for rewording docs or code comments, > those are always first validated using AI language tools. If I suggest > one phrasing is better than another it is not purely my personal > opinion >
Fair enough. But note that many-a-times AI's judgement is not correct. For example, here your original complaint was that the comment was repetitive and your suggested wording won't solve that complaint; rather it is a somewhat tighter version of the existing comment without adding any new information to it. As the errmsg+errdetail actually conveys what the comment wants to say in most except the one in ATSimplePermissions(). So, we could remove the comments from all places except the one in ATSimplePermissions() and keep a generic comment atop IsConflictLogTableNamespace/IsConflictLogTableClass on the following lines: /* * IsConflictLogTableNamespace * True iff namespace is pg_conflict. * * Does not perform any catalog accesses. * * Tables in the pg_conflict namespace are conflict log tables, created and * maintained by the system to record logical replication conflicts. The * apply worker's inserts depend on their fixed structure, so they are treated * as system-managed: call sites across the backend use this (and * IsConflictLogTableClass()) to reject operations that could disrupt logging, * such as DDL, manual INSERT/UPDATE/MERGE, and row locking. DELETE and * TRUNCATE are the only data operations allowed, so that users can still * prune old conflict rows. */ /* * IsConflictLogTableClass * True iff the relation is a conflict log table. * * Does not perform any catalog accesses. See * IsConflictLogTableNamespace() for why operations on these tables are * restricted. */ -- With Regards, Amit Kapila.
