Re: Proposal: Conflict log history table for Logical Replication

On Wed, 30 Sept 2026 at 18:53, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Wed, Sep 30, 2026 at 12:28 PM Narayanan Venkateswaran
> <narayananvpostgres(at)gmail(dot)com> wrote:
> >
> > Hi Nisha,
> >
> > Thank you very much for the guidance and the pointers to the older thread,
> >
> > Please find some replies inline,
> >
> > On Wed, Sep 30, 2026 at 11:05 AM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >>
> >> On Tue, Sep 29, 2026 at 5:56 PM Narayanan Venkateswaran
> >> <narayananvpostgres(at)gmail(dot)com> wrote:
> >> >
> >> > Thank you very much for the excellent work. I looked at the patch v77,
> >> >
> >>
> >> Hi Narayanan, thanks for reviewing it.
> >>
> >> > The code decides replica_identity_full using the following logic (in conflict.c),
> >> >
> >> > if (!TupIsNull(searchslot))
> >> > {
> >> > Oid replica_index = GetRelationIdentityOrPK(rel);
> >> >
> >> > /*
> >> > * If the table has a valid replica identity index, build the index
> >> > * JSON datum from key value. Otherwise, in REPLICA IDENTITY FULL
> >> > * cases, set replica_identity_full to true and leave replica_identity
> >> > * NULL to avoid serializing full tuples that could exceed memory
> >> > * allocation limits.
> >> > */
> >> > if (OidIsValid(replica_index))
> >> > {
> >> > values[attno++] = BoolGetDatum(false);
> >> > values[attno++] = build_index_key_json(rel,
> >> > replica_index,
> >> > searchslot,
> >> > &omitted);
> >> > }
> >> > else
> >> > {
> >> > values[attno++] = BoolGetDatum(true);
> >> > nulls[attno++] = true;
> >> > }
> >> > }
> >> > else
> >> > {
> >> > nulls[attno++] = true;
> >> > nulls[attno++] = true;
> >> > }
> >> >
> >> > In PostgreSQL catalogs (pg_class.relreplident), a table's replica can be one of four values:
> >> >
> >> > 'd' = REPLICA_IDENTITY_DEFAULT: Use PK index if one exists. If the table has no PK, it has no index and is NOT FULL.
> >> > 'n' = REPLICA_IDENTITY_NOTHING: No replica identity.
> >> > 'i' = REPLICA_IDENTITY_INDEX: Explicit unique index.
> >> > 'f' = REPLICA_IDENTITY_FULL: The entire tuple is the identity.
> >> >
> >> > If a subscriber relation has REPLICA IDENTITY DEFAULT without a primary key (or REPLICA IDENTITY NOTHING) GetRelationIdentityOrPK() returns InvalidOid. In this case, the code sets replica_identity_full = true.
> >> >
> >>
> >> I think there may be some misunderstanding about what the
> >> replica_identity_full column actually stores. This question was also
> >> raised earlier; please see [1] and the discussion that followed. This
> >> field indicates the subscriber’s actual search method.
> >
> >
> > Thank you for clarifying the intended design and also the pointer to the old thread.
> >
> > I understand now that the intention for `replica_identity_full` is to indicate whether the conflicting row was located via a specific replica key index (`false`) versus a full-tuple search (`true`), rather than reflecting the DDL catalog property (pg_class.relreplident).
> >
>
> I think the docs can be improved to avoid this confusion, as Vignesh
> also suggested earlier in [1]. How about updating it to:
> "Indicates whether the conflicting local row was located using the
> full tuple (true) or the replica identity key of the local table
> (false)."
>
> Let me know if this works for you.

The (true) and (false) in this wording are a little unclear, as it is
not obvious what they refer to. I'm not sure they are necessary here.
If we do want to mention the Boolean values, it may be clearer to
explicitly refer to the field, for example, replica_identity_full
(true/false).

Regards,
Vignesh