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 Shveta
