On Tue, Mar 14, 2017 at 7:17 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> > @@ -234,6 +236,21 @@ index_beginscan(Relation heapRelation, > > scan->heapRelation = heapRelation; > > scan->xs_snapshot = snapshot; > > > > + /* > > + * If the index supports recheck, make sure that index tuple is > saved > > + * during index scans. > > + * > > + * XXX Ideally, we should look at all indexes on the table and > check if > > + * WARM is at all supported on the base table. If WARM is not > supported > > + * then we don't need to do any recheck. > RelationGetIndexAttrBitmap() does > > + * do that and sets rd_supportswarm after looking at all indexes. > But we > > + * don't know if the function was called earlier in the session > when we're > > + * here. We can't call it now because there exists a risk of > causing > > + * deadlock. > > + */ > > + if (indexRelation->rd_amroutine->amrecheck) > > + scan->xs_want_itup = true; > > + > > return scan; > > } > > I didn't like this comment very much. But it's not necessary: you have > already given relcache responsibility for setting rd_supportswarm. The > only problem seems to be that you set it in RelationGetIndexAttrBitmap > instead of RelationGetIndexList, but it's not clear to me why. I think > if the latter function is in charge, then we can trust the flag more > than the current situation. I looked at this today. AFAICS we don't have access to rd_amroutine in RelationGetIndexList since we don't actually call index_open() in that function. Would it be safe to do that? I'll give it a shot, but thought of asking here first. Thanks, Pavan