public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Ashutosh Bapat <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Confused comment about drop replica identity index
Date: Wed, 15 Dec 2021 12:24:45 +0900
Message-ID: <Yblf/[email protected]> (raw)
In-Reply-To: <CAExHW5uWwcV0-=JQXYhEpnEtmwmZQsh0c1N3cxxBYRhwE9WuvQ@mail.gmail.com>
References: <OS3PR01MB6275464AD0A681A0793F56879E759@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAExHW5uWwcV0-=JQXYhEpnEtmwmZQsh0c1N3cxxBYRhwE9WuvQ@mail.gmail.com>

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/[email protected]

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


Attachments:

  [application/pgp-signature] signature.asc (833B, ../Yblf%[email protected]/2-signature.asc)
  download

view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Confused comment about drop replica identity index
  In-Reply-To: <Yblf/[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox