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 <[email protected]> wrote: > On Tue, Sep 29, 2026 at 5:56 PM Narayanan Venkateswaran > <[email protected]> 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). > > > 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. > However, I have the following question related to the following doc entry, + <row> + <entry><literal>replica_identity_full</literal></entry> + <entry><type>boolean</type></entry> + <entry>Indicates whether the conflicting relation uses <literal>REPLICA IDENTITY FULL</literal> (<literal>true</literal>) or a replica identity index (<literal>false</literal>). This is <literal>NULL</literal> when replica identity information is not applicable.</entry> + </row> The doc states it is "NULL when replica identity information is not applicable". However, in insert_conflict_log_tuple(), replica_identity_full is only set to NULL if TupIsNull(searchslot) is true. Since searchslot (remoteslot) is always populated for all currently logged conflicts, the outer else block is never reached and replica_identity_full is never NULL. Should the documentation be updated to remove the reference to NULL, or is there a case where searchslot can be empty ? > > [I would request you to please reply inline to keep the discussion > relevant and easier to follow.] > Really sorry, my humble apologies for the inconvenience. > > [1] > https://www.postgresql.org/message-id/CAFiTN-u8xH%2BLVVNx8OJxFnub5eHTWw9v7sCcffXtPKKQ1CG2Gw%40mail.gmail.com > -- > Thanks, > Nisha > Thank you, Narayanan
