On Thu, Jun 25, 2026 at 4:57 PM Amit Kapila <[email protected]> wrote: > > On Thu, Jun 25, 2026 at 10:34 AM vignesh C <[email protected]> wrote: > > > > On Wed, 24 Jun 2026 at 19:27, Dilip Kumar <[email protected]> wrote: > > > > > > > 2) jsonb supports indexing, whereas json does not. Was json chosen > > here because inserts are faster due to the lack of parsing and binary > > conversion? > > > > I think it is more because the local/remote tuples are usually data > which we want users to see as it was in original tables, using jsonb > can change the ordering of columns or remove some space etc. We can > add a comment on the lines of: "preserve the exact tuple > representation (column order, formatting) for the audit record; the > value is natively json so this avoids a per-conflict conversion". Now, > this is true for tuple data but I am not so sure if the same thing > applies to the replica_identity column in CLT. Would users ever want > to query using ORDER/GROUP BY on replica_identity or use DISTINCT on > it, because if that is possible then the current schema would result > into ERROR as follows: > postgres=# select * from pg_conflict.pg_conflict_log_16394 order by > replica_identity; > ERROR: could not identify an ordering operator for type json > > This needs more thoughts from the perspective of how users would like > to fetch the data. > > Few other comments: > * errdetail("Conflict log tables are system-managed and only support > cleanup via DELETE or TRUNCATE.") > > In the above message 'via' sounds a bit odd, can we instead use > 'using' aka "Conflict log tables are system-managed and only support > cleanup using DELETE or TRUNCATE."
Okay > 2. > @@ -2482,6 +2681,8 @@ DropSubscription(DropSubscriptionStmt *stmt, > bool isTopLevel) > deleteDependencyRecordsFor(SubscriptionRelationId, subid, false); > deleteSharedDependencyRecordsFor(SubscriptionRelationId, subid, 0); > > + drop_sub_conflict_log_table(subid, subname, subconflictlogrelid); > > It would be better to drop the table before cleaning up the dependency > record. Right now, it is okay even in current order because dependency > removal is trying to remove where subid is depender. So whats your suggestion change it now or not? I feel either way is fine. > Apart from above, attached contains a cosmetic/comment improvement patch. Okay, I will merge -- Regards, Dilip Kumar Google
