On Sat, Jul 22, 2023 at 7:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Jul 21, 2023 at 6:55 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached the updated patch. I'll push it early next week, barring
> > any objections.
> >
>
> You have moved most of the comments related to the restriction of
> which index can be picked atop IsIndexUsableForReplicaIdentityFull().
> Now, the comments related to limitation atop
> FindUsableIndexForReplicaIdentityFull() look slightly odd as it refers
> to limitations but those limitation were not stated. The comments I am
> referring to are: "Note that the limitations of index scans for
> replica identity full only .... might not be a good idea in some
> cases". Shall we move these as well atop
> IsIndexUsableForReplicaIdentityFull()?
Good point.
Looking at neighbor comments, the following comment looks slightly odd to me:
* XXX: See IsIndexUsableForReplicaIdentityFull() to know the challenges in
* supporting indexes other than btree and hash. For partial indexes, the
* required changes are likely to be larger. If none of the tuples satisfy
* the expression for the index scan, we fall-back to sequential execution,
* which might not be a good idea in some cases.
Are the first and second sentences related actually?
I think we can move it as well to
IsIndexUsableForReplicaIdentityFull() with some adjustments. I've
attached the updated patch that incorporated your comment and included
my idea to update the comment.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com