Re: Confused comment about drop replica identity index

On Tue, Dec 14, 2021 at 6:08 PM wangw(dot)fnst(at)fujitsu(dot)com
<wangw(dot)fnst(at)fujitsu(dot)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