On Thu, 1 Oct 2026 at 10:26, shveta malik <[email protected]> wrote: > > On Thu, Oct 1, 2026 at 10:17 AM vignesh C <[email protected]> wrote: > > > > On Wed, 30 Sept 2026 at 18:53, Nisha Moond <[email protected]> wrote: > > > > > > On Wed, Sep 30, 2026 at 12:28 PM Narayanan Venkateswaran > > > <[email protected]> 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 <[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). > > > > > > > > > > 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). > > Shall it be: > > "True if the conflicting local row was located using the full tuple, > rather than the replica identity key of the local table."
Thanks, this looks good to me. Regards, Vignesh
