Re: Proposal: Conflict log history table for Logical Replication

Thanks for v56. A few comments:

0001:
1)
+ * We complain if either the old or new namespaces is a temporary schema,
+ * temporary toast schema, the TOAST schema, or the conflict schema.
*/
void
CheckSetNamespace(Oid oldNspOid, Oid nspOid)

Shall we remove this comment change now as the check has been removed?

2)
+ /*
+ * Conflict log tables are managed by the system to record logical
+ * replication conflicts.
+ */
+ if (IsConflictLogTableNamespace(RelationGetNamespace(rel)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in the conflict log table \"%s\"",
+ RelationGetRelationName(rel))));

postgres=# SELECT * FROM pg_conflict.pg_conflict_log_16387 for UPDATE;
ERROR: cannot lock rows in the conflict log table "pg_conflict_log_16387"

Why don't we add detail-message here as well, similar to all other cases:
errdetail("Conflict log tables are system-managed tables for logical
replication conflicts.")));

patch-0002
3)
+ SELECT 'pg_conflict.' || relname INTO tab_name
+ FROM pg_class c JOIN pg_subscription s ON c.relname =
'pg_conflict_log_' || s.oid
+ WHERE s.subname = 'regress_conflict_protection_test';

This query runs many times (10-15). Can we save the result into
clt_name and use it everywhere? This will reduce test-size.

SELECT 'pg_conflict.' || relname AS clt_name
FROM pg_class c JOIN pg_subscription s ON c.relname =
'pg_conflict_log_' || s.oid
WHERE s.subname = 'regress_conflict_protection_test'
\gset

DO
BEGIN
EXECUTE 'CREATE TABLE public.conflict_child () INHERITS (' ||
:'clt_name' || ')';
....

DO
BEGIN
EXECUTE 'ALTER TABLE ' || :'clt_name' || ' RENAME COLUMN relname
TO new_relname';
---

4)
We can also add a test for paritioning along with inheritance:
CREATE TABLE clt_partition PARTITION OF
pg_conflict.pg_conflict_log_16387 FOR VALUES FROM ('2026-01-01') TO
('2027-01-01');

thanks
Shveta