Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

From Nisha Moond
Subject Re: Proposal: Conflict log history table for Logical Replication
Date
Msg-id CABdArM5=92GOFPVPEeq0k66-ybvCC3_G2c7qCBu+qb7kRg_ntA@mail.gmail.com
Whole thread
In response to Re: Proposal: Conflict log history table for Logical Replication  (Narayanan Venkateswaran <narayananvpostgres@gmail.com>)
Responses Re: Proposal: Conflict log history table for Logical Replication
List pgsql-hackers
On Tue, Sep 29, 2026 at 5:56 PM Narayanan Venkateswaran
<narayananvpostgres@gmail.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
informationis 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
incomingrow 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_fullto 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



pgsql-hackers by date:

Previous
From: shihao zhong
Date:
Subject: REPACK (CONCURRENTLY): do not block the table while waiting for the final lock
Next
From: Haibo Yan
Date:
Subject: Re: Costing for parallel scans with few/single row produced in the outer side