Re: WIP: Access method extendability

On Tue, Mar 22, 2016 at 11:53 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:

> Alexander Korotkov wrote:
> > On Tue, Mar 22, 2016 at 1:26 AM, Alvaro Herrera <
> alvherre(at)2ndquadrant(dot)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