Pavan Deolasee wrote: > On Tue, Mar 14, 2017 at 7:17 AM, Alvaro Herrera <alvherre@2ndquadrant.com> > wrote:
> > I have already commented about the executor involvement in btrecheck(); > > that doesn't seem good. I previously suggested to pass the EState down > > from caller, but that's not a great idea either since you still need to > > do the actual FormIndexDatum. I now think that a workable option would > > be to compute the values/isnulls arrays so that btrecheck gets them > > already computed. > > I agree with your complaint about modularity violation. What I am unclear > is how passing values/isnulls array will fix that. The way code is > structured currently, recheck routines are called by index_fetch_heap(). So > if we try to compute values/isnulls in that function, we'll still need > access EState, which AFAIU will lead to similar violation. Or am I > mis-reading your idea?
You're right, it's still a problem.
BTW I realised that we don't really need those executor bits in recheck routines. We don't support WARM when attributes in index expressions are modified. So we really don't need to do any comparison for those attributes. I've written a separate form of FormIndexDatum() which will only return basic index attributes and comparing them should be enough. Will share rebased and updated patch soon.
