Re: TABLESAMPLE patch is really in pretty sad shape

On 2015-07-16 15:59, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
>> Petr Jelinek wrote:
>>> On 2015-07-13 00:36, Tom Lane wrote:
>>>> PS: now that I've written this rant, I wonder why we don't redesign the
>>>> index AM API along the same lines. It probably doesn't matter much at
>>>> the moment, but if we ever get serious about supporting index AM
>>>> extensions, I think we ought to consider doing that.
>
>>> I think this is very relevant to the proposed sequence am patch as well.
>
>> Hmm, how would this work? Would we have index AM implementation run
>> some function that register their support methods somehow at startup?
>
> Well, registration of new index AMs is an unsolved question ATM anyhow.
> But what I'm imagining is that pg_am would reduce to about two columns,
> amname and a handler function OID, and everything else that constitutes
> the API for AMs would get moved down to the C level. We have to keep that
> catalog because we still need index AMs to have OIDs that will represent
> them in pg_opclass etc; but we don't need to nail the exact set of AM
> interface functions into the catalog. (I'm not sure whether we'd want
> to remove all the bool columns from pg_am. At the C level it would be
> about as convenient to have them in a struct returned by the handler
> function. But it's occasionally useful to have those properties
> visible to SQL queries.)

This is along the lines of how I was thinking also (when I read your
previous email). I think the properties of the index will have to be
decided on individual basis once somebody actually starts working on
this. But functions can clearly go into C struct if they are called only
from C anyway.

> I'm not clear on whether sequence AMs would need explicit catalog
> representation, or could be folded down to just a single SQL function
> with special signature as I suggested for tablesample handlers.
> Is there any need for a sequence AM to have additional catalog
> infrastructure like index AMs need?
>

That depends on the route we will choose to take with the storage there.
If we allow custom columns for sequence AMs (which is what both Heikki
and me seem to be inclined to do) then I think it will still need
catalog, plus it's also easier to just reuse the relam behavior than
coming up up with something completely new IMHO.

>> In any case, if indexes AMs and sequence AMs go this route, that
>> probably means the column store AM we're working on will probably have
>> to go the same route too.
>
> It's worth considering anyway. The FDW API has clearly been far more
> successful than the index AM API in terms of being practically usable
> by extensions.
>

Yep, I now consider it to be a clear mistake that I modeled both
sequence am and tablesample after indexes given that I targeted both at
extensibility.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services