Re: Confused comment about drop replica identity index

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Confused comment about drop replica identity index
Date: 2021-12-15 03:24:45
Message-ID: Yblf/R3h0Dj+Ui2v@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 14, 2021 at 07:10:49PM +0530, Ashutosh Bapat wrote:
> 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;

Yeah, the comment is wrong. If the index of a REPLICA_IDENTITY_INDEX
is dropped, I recall that the behavior is the same as
REPLICA_IDENTITY_NOTHING.

> 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.

Not sure about the DROP INDEX page, but I'd be fine with mentioning
that in the ALTER TABLE page in the paragraph related to REPLICA
IDENTITY. While on it, I would be tempted to switch this stuff to use
a list of <variablelist> for all the option values. That would be
much easier to read.

[ ... thinks a bit ... ]

FWIW, this brings back some memories, as of this thread:
https://www.postgresql.org/message-id/20200522035028.GO2355@paquier.xyz

See also commit fe7fd4e from August 2020, where some tests have been
added. I recall seeing this incorrect comment from last year's
thread and it may have been mentioned in one of the surrounding
threads.. Maybe I just let it go back then. I don't know.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2021-12-15 03:34:56 Re: row filtering for logical replication
Previous Message Kyotaro Horiguchi 2021-12-15 03:01:57 Re: Is it worth adding ReplicationSlot active_pid to ReplicationSlotPersistentData?