MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Thu, 14 May 2020 18:30:38 +0300
From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: daniel@yesql.se, a.lakhin@postgrespro.ru, pgsql-docs@postgresql.org
Subject: Re: Missing comma?
In-Reply-To: <20200514000738.GA166343@paquier.xyz>
References: <ad1a0badc32658b1bbb07aa312346a1d@postgrespro.ru>
 <5B3957FE-D31E-4838-B35C-C2F0EEC3E557@yesql.se>
 <6fa8ba75057b16f88010c2b945f720c0@postgrespro.ru>
 <20200514000738.GA166343@paquier.xyz>
User-Agent: Roundcube Webmail/1.4.0
Message-ID: <226cae2ef0bf26a59011de38eec9c7f4@postgrespro.ru>
Organization: Postgres Professional
Precedence: bulk

On 2020-05-14 03:07, Michael Paquier wrote:
> On Wed, May 13, 2020 at 05:17:43PM +0300, Marina Polyakova wrote:
>> in my opinion it's a little unclear what "or default" means at the 
>> end,
>> because the comma is used to separate enumeration elements ("d = 
>> default
>> <...>, n = nothing, f = all columns, i = index <...>") and inside the
>> element description ("i = index with indisreplident set, or default").
>> Therefore here is an additional patch from me to clarify this place, 
>> thanks
>> to Alexander Lakhin for help.
> 
> Yes, I agree that this last "or default" in the docs does not make
> much sense, because we always enforce REPLICA_IDENTITY_NOTHING if
> there is no replica identity.

This looks like a shortened version of a comment from 
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'

>> diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
>> index 
>> 02ddebae99d98110a8dd290dd4cb0c980adf7984..034a08f80ea4269f131e7e1383ba482fd76d9344 
>> 100644
>> --- a/doc/src/sgml/catalogs.sgml
>> +++ b/doc/src/sgml/catalogs.sgml
>> @@ -1936,7 +1936,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration 
>> count&gt;</replaceable>:<replaceable>&l
>>         <literal>d</literal> = default (primary key, if any),
>>         <literal>n</literal> = nothing,
>>         <literal>f</literal> = all columns,
>> -       <literal>i</literal> = index with 
>> <structfield>indisreplident</structfield> set, or default
>> +       <literal>i</literal> = index with 
>> <structfield>indisreplident</structfield> set (if any)
>>        </entry>
>>       </row>
> 
> And you don't need the ("if any") either here, no?
> REPLICA_IDENTITY_INDEX means that we have an index associated with the
> replica identity so it seems to me that this last part can just be
> removed.

I tried to save information for cases when the index is dropped as in 
the code comment (see above) and the function RelationGetIndexList 
(where IIUC we check if this index exists):

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;

Perhaps it will be useful for some users to know that relreplident = i 
does not mean that the required index exists (as in default case)?..

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company