Re: Proposal: Conflict log history table for Logical Replication

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.

> However, the SGML docs update in the patch states that replica_identity_full "is NULL when replica identity information is not applicable".
>
> The only way replica_identity_full can ever be set to NULL is if the execution enters the outer else block: when TupIsNull(searchslot) is true (i.e., searchslot is NULL or empty). However, it looks like this slot contains the incoming row data sent by the publisher. It is always populated and never null.
>
> Because searchslot is never null, the outer else block is never executed. The code will never set replica_identity_full to NULL.
>
> I think it is better to explicitly check for REPLICA_IDENTITY_FULL in an else if block, something like the below,
>
> else if (rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL)
> {
> values[attno++] = BoolGetDatum(true);
> nulls[attno++] = true;
> }
>

As per [1], replica_identity_full value is determined independently of
relreplident, so I don't think we need this else-if branch.

[I would request you to please reply inline to keep the discussion
relevant and easier to follow.]

[1] https://www.postgresql.org/message-id/CAFiTN-u8xH%2BLVVNx8OJxFnub5eHTWw9v7sCcffXtPKKQ1CG2Gw%40mail.gmail.com
--
Thanks,
Nisha