On Mon, Jul 24, 2023 at 12:05 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Jul 24, 2023 at 6:39 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Sat, Jul 22, 2023 at 7:32 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > > > > 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? > > > > Not really. > > > 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. > > > > LGTM.
Pushed. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com