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(at)gmail(dot)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