Re: Proposal: Conflict log history table for Logical Replication

On Thu, Oct 15, 2026 at 3:22 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Wed, Oct 14, 2026 at 8:16 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > On Tue, Oct 13, 2026 at 3:34 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > I merged v80-0001 and v80-0002 and adjusted the commit message and
> > > test accordingly; I also included Amit's and Hou-san's code
> > > refactoring. Rebased Nisha's V80_2-0003 on top of that as 0002, also
> > > merged Shveta's datestyle fix with some modification into the 0002
> > >
> >
> > Thanks for the updated patches. A few comments on v81-0001:
> >
>
> There seems to be a bug where the replica_identity column uses index
> attribute names, which can be stale or incorrect.
> build_index_key_json() takes each JSON key name from the index's tuple
> descriptor:
>
> - tableslot = ExecCopySlot(tableslot, slot);
> + Form_pg_attribute att = TupleDescAttr(indexTupDesc, i);
> +
> + if (i > 0)
> + appendStringInfoChar(&result, ',');
> +
> + escape_json(&result, NameStr(att->attname));
>
> Index attribute names are copied from the table when the index is
> created, and aren't kept in sync afterwards. So the CLT can record a
> key under a name that isn't the column's current name. The server log
> doesn't have this problem: BuildIndexValueDescription() builds the
> names with pg_get_indexdef_columns(), which reads the current column
> names from the table. So the CLT and the log disagree in below two
> cases.
>
> Case 1: Renamed key column: the CLT records the old name
> ----------------------------------------------
> -- [PUB]
> CREATE TABLE r3 (a int PRIMARY KEY, b int);
> CREATE PUBLICATION pub3 FOR TABLE r3;
>
> -- [SUB]
> CREATE TABLE r3 (a int PRIMARY KEY, b int);
> CREATE SUBSCRIPTION sub3 CONNECTION '...' PUBLICATION pub3 WITH
> (copy_data = false, conflict_log_destination = 'all');
>
> -- [PUB]
> INSERT INTO r3 VALUES (1, 10);
> ALTER TABLE r3 RENAME COLUMN a TO z;
>
> -- [SUB]
> ALTER TABLE r3 RENAME COLUMN a TO z;
> DELETE FROM r3;
>
> -- [PUB] -- trigger conflict
> DELETE FROM r3 WHERE z = 1;
>
> CLT:
> postgres=# select conflict_type, replica_identity from
> pg_conflict.pg_conflict_log_16390;
> conflict_type | replica_identity
> ----------------+------------------
> delete_missing | {"a":"1"}
>
> Meanwhile the LOG shows the correct altered name:
> LOG: conflict detected on relation "public.r3": conflict=delete_missing
> DETAIL: Could not find the row to be deleted: replica identity (z)=(1).
> ~~~
>
> case 2: UNIQUE (a, a) replica identity: a value is recorded under
> another column's name
> ----------------------------------------------
> -- [PUB]
> CREATE TABLE d3 (a int PRIMARY KEY, a1 int, b int);
> CREATE PUBLICATION pub3d FOR TABLE d3;
>
> -- [SUB]
> CREATE TABLE d3 (a int NOT NULL, a1 int NOT NULL, b int);
> CREATE UNIQUE INDEX d3_aa ON d3 (a, a);
> ALTER TABLE d3 REPLICA IDENTITY USING INDEX d3_aa;
> CREATE SUBSCRIPTION sub3d CONNECTION '...' PUBLICATION pub3d WITH
> (copy_data = false, conflict_log_destination = 'all');
>
> -- [PUB]
> INSERT INTO d3 VALUES (1, 99, 0);
>
> -- [SUB]
> DELETE FROM d3;
>
> -- [PUB]
> DELETE FROM d3;
>
> CLT:
> postgres=# select conflict_type, replica_identity from
> pg_conflict.pg_conflict_log_16406;
> conflict_type | replica_identity
> ----------------+--------------------
> delete_missing | {"a":"1","a1":"1"}
> (1 row)
>
> LOG shows:
> LOG: conflict detected on relation "public.d3": conflict=delete_missing
> DETAIL: Could not find the row to be deleted: replica identity (a, a)=(1, 1).
> ~~~
>
> How about fixing it like in the attached patch.

Okay, I agree with the fix. Basically now, instead of relying on the
index’s own copy of the column names (which can go stale, as it isn’t
refreshed when the table’s schema changes), we now take the column
number from the index (which never goes stale) and use the relation
descriptor to fetch the column's info.

thanks
Shveta