On Tue, Mar 22, 2016 at 11:53 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> Alexander Korotkov wrote: > > On Tue, Mar 22, 2016 at 1:26 AM, Alvaro Herrera < > alvhe...@2ndquadrant.com> > > wrote: > > > > I noticed this state of affairs because I started reading the complete > > > thread in order to verify whether a consensus had been reached > regarding > > > the move of IndexQualInfo and GenericCosts to selfuncs.h. But I only > > > see that Jim Nasby mentioned it and Alexander said "yes they move"; > > > nothing else. The reason for raising this is that, according to > > > Alexander, new index AMs will want to use those structs; but current > > > index AMs only include index_selfuncs.h, and none of them includes > > > selfuncs.h, so it seems completely wrong to be importing all the > planner > > > knowledge embodied in selfuncs.h into extension index AMs. > > > > > > One observation is that the bloom AM patch (0003 of this series) uses > > > GenericCosts but not IndexQualInfo. But btcostestimate() and > > > gincostestimate() both use IndexQualInfo, so other AMs might well want > > > to use it too. > > > > > > We could move GenericCosts and the prototype for genericcostestimate() > > > to index_selfuncs.h; that much is pretty reasonable. But I'm not sure > > > what to do about IndexQualInfo. We could just add forward struct > > > declarations for RestrictInfo and Node to index_selfuncs.h. But then > > > the extension code is going to have to import the includes were those > > > are defined anyway. Certainly it seems that deconstruct_indexquals() > > > (which returns a list of IndexQualInfo) is going to be needed by > > > extension index AMs, at least ... > > > > Thank you for highlighting these issues. > > After thinking some more about this, I think it's all right to move both > IndexQualInfo and GenericCosts to selfuncs.h. The separation that we > want is this: the selectivity functions themselves need access to a lot > of planner knowledge, and so selfuncs.h knows a lot about planner. But > the code that _calls_ the selectivity function doesn't; and > index_selfuncs.h is about the latter. Properly architected extension > index AMs are going to have their selectivity functions in a separate .c > file which knows a lot about planner, and which includes selfuncs.h (and > maybe index_selfuncs.h if it wants to piggyback on the existing > selecticity functions, but that's unlikely); only the prototype for the > selfunc is going to be needed elsewhere, and the rest of the index code > is not going to know anything about planner stuff so it will not need to > include selfuncs.h nor index_selfuncs.h. Right, the purposes of headers are: * selfuncs.h – for those who going to estimate selectivity, * index_selfuncs.h – for those who going to call selectivity estimation functions of built-in AMs. >From this point for view we should keep both IndexQualInfo and GenericCosts in selfuncs.h. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company