Hi Dilip, Please find a few comments on v19-001: 1) We can replace IsConflictLogTable(relid) with IsConflictClass(reltuple). Wherever we call IsConflictLogTable(), we have already fetched reltuple from pg_class for that relid, we can simply use that to see if it belongs to pg_conflict namespace. That will avoid the need of patch004 as well to 'Introduce a dedicated shared unique index on pg_subscription.subconflictlogrelid'.
2) + if (opts.logdest == CONFLICT_LOG_DEST_TABLE || + opts.logdest == CONFLICT_LOG_DEST_ALL) We can replace with: IsSet(opts.logdest, CONFLICT_LOG_DEST_TABLE); 3) +-- this should generate an internal table named pg_conflict_$subid$ +CREATE SUBSCRIPTION regress_conflict_test1 CONNECTION 'dbname=regress_doesnotexist' PUBLICATION testpub WITH (connect = false, conflict_log_destination = 'table'); + I think we shall verify 2 things here: a) Table is created in pg_conflict namespace. b) Table has name pg_conflict_<subid> 4) We can add these 3 simple tests as well: a) Trying to alter and truncate pg_conflict_subid table. b) Trying to create a new table in pg_conflict namespace. c) Moving a table into pg_conflict namespace. ~~ Overall, I’m concerned about how users will manage this table as it grows. There is currently no way to purge old data, truncation is disallowed, and the table must be sub-ID–tied, which also prevents users from attaching a different table as a CLT (if needed). Additionally, we do not offer any form of partitioning. Do you think we should provide users with a basic purge mechanism? At the very least, should we allow truncation so users can take a backup and truncate a sub-ID–tied CLT to start afresh? Thoughts? thanks Shveta
