Re: Proposal: Conflict log history table for Logical Replication

On Mon, May 25, 2026 at 10:13 AM vignesh C <vignesh21@gmail.com> wrote:
>
>
> Thanks for the comments, the attached v39 version patch has the
> changes for the same.
>

I have not yet looked at v40, but please find a few ocmments on
v39-0001 and 0002 merged together.

1)
heap_create:
+ errdetail("Conflict schema modifications are currently disallowed.")));
LookupCreationNamespace:
+ errmsg("cannot move objects into or out of the pg_conflict schema")));

Can we make it same through-out, either we use 'Conflict schema' at
both the places or pg_conflict schema.  Since in these 2 functions, in
previous messages, we are using names like 'System catalog', 'TOAST
schema' etc, I think we can use Conflict schema at both the places.
What do others think on this?


2)
drop_subscription_dependencies():

+ conflictrelname = get_rel_name(subconflictlogrelid);

We can actually have a sanity check that we got the CLT using the relid.
Assert(conflictrelname != NULL);

3)
+ /*
+ * Special handling for the JSON array type for proper
+ * TupleDescInitEntry call.
+ */
+ if (type_oid == JSONARRAYOID)
+ type_oid = get_array_type(JSONOID);

Why do we have this special handling? Do we expect that 'type_oid' can
be different from JSONARRAYOID if we use get_array_type? On debugging,
I found it to be same pre and post get_array_type()

4)
Do we need to have CommandCounterIncrement() after
heap_create_with_catalog() in create_conflict_log_table()? I think
even if we are not doing any table_open etc for CLT in same
transaction, we should call CommandCounterIncrement() (to be
consistent with other such calls of heap_create_with_catalog and to
make it future proof). Thoughts?

thanks
Shveta