Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

On Wed, 8 Mar 2023 at 21:46, Önder Kalacı <onderkalaci@gmail.com> wrote:
>
> Hi Vignesh C,
>
>>
>>
>> Few comments
>> 1) Maybe this change is not required:
>>     fallback if no other solution is possible.  If a replica identity other
>>     than <quote>full</quote> is set on the publisher side, a replica identity
>> -   comprising the same or fewer columns must also be set on the subscriber
>> -   side.  See <xref linkend="sql-altertable-replica-identity"/> for details on
>> +   comprising the same or fewer columns must also be set on the
>> subscriber side.
>> +   See <xref linkend="sql-altertable-replica-identity"/> for details on
>
>
> Yes, fixed.
>>
>>
>> 2) Variable declaration and the assignment can be split so that the
>> readability is better:
>> +
>> +               bool            isUsableIndex =
>> +                       IsIndexUsableForReplicaIdentityFull(indexInfo);
>> +
>> +               index_close(indexRelation, AccessShareLock);
>> +
>
>
> Hmm, can you please elaborate more on this? The declaration
> and assignment are already on different lines.
>
> ps: pgindent changed this line a bit. Does that look better?

I thought of changing it to something like below:
bool isUsableIndex;
Oid idxoid = lfirst_oid(lc);
Relation indexRelation = index_open(idxoid, AccessShareLock);
IndexInfo  *indexInfo = BuildIndexInfo(indexRelation);

isUsableIndex = IsIndexUsableForReplicaIdentityFull(indexInfo);

Regards,
Vignesh