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?
3) Since there is only one statement within the if condition, the
> braces can be removed
> + if (is_btree && !is_partial && !is_only_on_expression)
> + {
> + return true;
> + }
>
>
Fixed on a newer version of the patch. Now it is only:
*return is_btree && !is_partial && !is_only_on_expression;*
> 4) There is minor indentation issue in this, we could run pgindent to fix
> it:
> +static Oid FindLogicalRepLocalIndex(Relation localrel,
> +
> LogicalRepRelation *remoterel);
> +
>
>
Yes, pgindent fixed it, thanks.
Attached v37
Thanks,
Onder KALACI
v37_0002_use_index_on_subs_when_pub_rep_ident_full.patch
Description: Binary data
v37_0001_use_index_on_subs_when_pub_rep_ident_full.patch
Description: Binary data
