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. -- With Regards, Amit Kapila.
