| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-08-26 10:45:14 |
| Message-ID: | CAJpy0uBedr+aofTYv2JO0t_k7AUKL2EEYAXwFAp31gtJO+RcLQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Aug 26, 2026 at 3:34 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 24 Aug 2026 at 21:01, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > Yes this makes sense. Here is the new version of patches which usage
> > v71_1-0001 as base patch and v71-0002 is rebased on top of that and
> > the the ommitting the large column patch is implemented as a top up
> > patch
>
> Incorrect replica_identity_full value in conflict log table. There
> appears to be an issue in insert_conflict_log_tuple() when determining
> the representation of the local replica identity. The current
> implementation uses GetRelationIdentityOrPK() to distinguish REPLICA
> IDENTITY FULL from an index-based replica identity:
> + Oid replica_index =
> GetRelationIdentityOrPK(rel);
> +
> + /*
> + * If the table has a valid replica identity index,
> build the index
> + * JSON datum from key value. Otherwise, construct it from the
> + * complete tuple in REPLICA IDENTITY FULL cases.
> + */
> + if (OidIsValid(replica_index))
> + {
> + values[attno++] = BoolGetDatum(false);
> + values[attno++] =
> tuple_table_slot_to_indextup_json(estate, rel,
> +
>
> replica_index,
> +
> searchslot);
> + }
> + else
> + {
> + values[attno++] = BoolGetDatum(true);
> + values[attno++] =
> tuple_table_slot_to_json_datum(searchslot);
> + }
>
> The problem is that GetRelationIdentityOrPK() returns InvalidOid for
> three different replica identity states, not only for REPLICA IDENTITY
> FULL:
> relreplident = 'f' — REPLICA IDENTITY FULL
> relreplident = 'n' — REPLICA IDENTITY NOTHING
> relreplident = 'd' — REPLICA IDENTITY DEFAULT with no primary key
>
> As a result, all three cases enter the else branch and
> replica_identity_full is set to true. Thus, the replica_identity_full
> column does not actually indicate whether the relation is configured
> with REPLICA IDENTITY FULL; it instead indicates that
> GetRelationIdentityOrPK() did not find a replica identity index.
>
> This can be reproduced with the following test:
> Publisher
> CREATE TABLE tab_full (a int, b text);
> ALTER TABLE tab_full REPLICA IDENTITY FULL;
> CREATE TABLE tab_nothing (a int, b text);
> ALTER TABLE tab_nothing REPLICA IDENTITY NOTHING;
> CREATE TABLE tab_nopk (a int, b text);
> CREATE TABLE tab_pk (a int PRIMARY KEY, b text);
> INSERT INTO tab_full VALUES (1, 'from publisher');
> INSERT INTO tab_nothing VALUES (1, 'from publisher');
> INSERT INTO tab_nopk VALUES (1, 'from publisher');
> INSERT INTO tab_pk VALUES (1, 'from publisher');
> CREATE PUBLICATION pub FOR ALL TABLES;
>
> Subscriber
> CREATE TABLE tab_full (a int, b text);
> ALTER TABLE tab_full REPLICA IDENTITY FULL;
> CREATE TABLE tab_nothing (a int, b text);
> ALTER TABLE tab_nothing REPLICA IDENTITY NOTHING;
> CREATE TABLE tab_nopk (a int, b text);
> CREATE TABLE tab_pk (a int PRIMARY KEY, b text);
> CREATE SUBSCRIPTION ... PUBLICATION pub;
>
> Generate a local origin on the subscriber:
> UPDATE tab_full SET b = b;
> UPDATE tab_nothing SET b = b;
> UPDATE tab_nopk SET b = b;
> UPDATE tab_pk SET b = b;
>
> Then generate conflicts from the publisher:
> UPDATE tab_full SET b = 'from subscriber conflict';
> UPDATE tab_nothing SET b = 'from subscriber conflict';
> UPDATE tab_nopk SET b = 'from subscriber conflict';
> UPDATE tab_pk SET b = 'from subscriber conflict';
How the updates on tab_nothing and tab_nopk will go through if there
are no RIs on these on pub? Do you mean we need to have RI on pub
while no RI on sub? Can you give exact steps if so? This testcase
currently fails with:
postgres=# UPDATE tab_nothing SET b = 'from subscriber conflict';
ERROR: cannot update table "tab_nothing" because it does not have a
replica identity and publishes updates
> The local replica identity configuration is:
> SELECT relname, relreplident FROM pg_class WHERE relname IN
> ('tab_full', 'tab_nothing', 'tab_nopk', 'tab_pk');
> relname | relreplident
> -------------+--------------
> tab_full | f
> tab_nothing | n
> tab_nopk | d
> tab_pk | d
> (4 rows)
>
> However, the conflict log records:
> select relname, replica_identity_full from pg_conflict.pg_conflict_log_16395;
> relname | replica_identity_full
> -------------+-----------------------
> tab_full | t
> tab_nothing | t
> tab_nopk | t
> tab_pk | f
> (4 rows)
>
> The tab_nothing and tab_nopk entries demonstrate the issue: although
> their local replica identities are NOTHING and DEFAULT respectively,
> both are reported with replica_identity_full = true.
>
> I think the code should explicitly check relreplident (or otherwise
> distinguish REPLICA IDENTITY FULL from the other cases) rather than
> using InvalidOid from GetRelationIdentityOrPK() as an indication of
> REPLICA IDENTITY FULL. In particular, replica_identity_full should be
> true only when the local relation actually has REPLICA IDENTITY FULL.
>
> Additionally, should we update documentation for REPLICA IDENTITY
> NOTHING and DEFAULT here?
> + <row>
> + <entry><literal>replica_identity_full</literal></entry>
> + <entry><type>boolean</type></entry>
> + <entry>Indicates whether <literal>replica_identity</literal>
> represents a full tuple (<literal>true</literal>) or key values of a
> replica identity index (<literal>false</literal>). This is
> <literal>NULL</literal> when <literal>replica_identity</literal> is
> not logged.</entry>
> + </row>
>
> Regards,
> Vignesh
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jelte Fennema-Nio | 2026-08-26 10:55:57 | Re: PSA: Planning to grease protocol connections during 19beta |
| Previous Message | Amit Kapila | 2026-08-26 10:21:05 | Re: Assertion failure in GetSubscriptionRelations() with concurrent DROP TABLE |