Re: Confused comment about drop replica identity index - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Confused comment about drop replica identity index
Date
Msg-id CAExHW5uWwcV0-=JQXYhEpnEtmwmZQsh0c1N3cxxBYRhwE9WuvQ@mail.gmail.com
Whole thread Raw
In response to Confused comment about drop replica identity index  ("wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>)
Responses Re: Confused comment about drop replica identity index
List pgsql-hackers
On Tue, Dec 14, 2021 at 6:08 PM wangw.fnst@fujitsu.com
<wangw.fnst@fujitsu.com> wrote:
>
> Hi hackers,
>
> When I doing development based by PG, I found the following comment have a
> little problem in file src/include/catalog/pg_class.h.
>
> /*
>  * an explicitly chosen candidate key's columns are used as replica identity.
>  * Note this will still be set if the index has been dropped; in that case it
>  * has the same meaning as 'd'.
>  */
> #define           REPLICA_IDENTITY_INDEX        'i'
>
> The last sentence makes me a little confused :
> [......in that case it as the same meaning as 'd'.]
>
> Now, pg-doc didn't have a clear style to describe this.
>
>
> But if I drop relation's replica identity index like the comment, the action
> is not as same as default.
>
> For example:
> Execute the following SQL:
> create table tbl (col1 int  primary key, col2 int not null);
> create unique INDEX ON tbl(col2);
> alter table tbl replica identity using INDEX tbl_col2_idx;
> drop index tbl_col2_idx;
> create publication pub for table tbl;
> delete from tbl;
>
> Actual result:
> ERROR:  cannot delete from table "tbl" because it does not have a replica identity and publishes deletes
> HINT:  To enable deleting from the table, set REPLICA IDENTITY using ALTER TABLE.

I think I see where's the confusion. The table has a primary key and
so when the replica identity index is dropped, per the comment in
code, you expect that primary key will be used as replica identity
since that's what 'd' or default means.

>
> Expected result in comment:
> DELETE 0
>
>
> I found that in the function CheckCmdReplicaIdentity, the operation described
> in the comment is not considered,
> When relation's replica identity index is found to be InvalidOid, an error is
> reported.

This code in RelationGetIndexList() is not according to that comment.

   if (replident == REPLICA_IDENTITY_DEFAULT && OidIsValid(pkeyIndex))
        relation->rd_replidindex = pkeyIndex;
    else if (replident == REPLICA_IDENTITY_INDEX && OidIsValid(candidateIndex))
        relation->rd_replidindex = candidateIndex;
    else
        relation->rd_replidindex = InvalidOid;

>
> Are the comment here not accurate enough?
> Or we need to adjust the code according to the comments?
>

Comment in code is one thing, but I think PG documentation is not
covering the use case you tried. What happens when a replica identity
index is dropped has not been covered either in ALTER TABLE
https://www.postgresql.org/docs/13/sql-altertable.html or DROP INDEX
https://www.postgresql.org/docs/14/sql-dropindex.html documentation.


-- 
Best Wishes,
Ashutosh Bapat



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?
Next
From: Ashutosh Bapat
Date:
Subject: Re: more descriptive message for process termination due to max_slot_wal_keep_size