Re: WIP: Access method extendability - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: WIP: Access method extendability
Date
Msg-id CAPpHfduVhOc7yRu1FxzszX8Ma=JF6u2xXCCo7+mMOfHnyj4rmQ@mail.gmail.com
Whole thread Raw
In response to Re: WIP: Access method extendability  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
On Tue, Mar 22, 2016 at 11:53 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Alexander Korotkov wrote:
> On Tue, Mar 22, 2016 at 1:26 AM, Alvaro Herrera <alvherre@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
 

pgsql-hackers by date:

Previous
From: Petr Jelinek
Date:
Subject: Re: Reworks of CustomScan serialization/deserialization
Next
From: Ashutosh Sharma
Date:
Subject: Re: Performance degradation in commit 6150a1b0