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
