On 2015-09-25 17:45, Alvaro Herrera wrote:
>
> I think the API of getOpFamilyInfo is a bit odd; is the caller expected
> to fill intype and keytype always, and then it only sets the procs/opers
> lists? I wonder if it would be more sensible to have that routine
> receive the pg_opclass tuple (or even the opclass OID) instead of the
> opfamily OID.
>
> I think "amapi.h" is not a great file name. What about am_api.h?
>
Well we have related fdwapi.h and tsmapi.h (and several unrelated *api.h
which also don't use "_" in the name) so amapi.h seems fine to me.
> I'm unsure about BRIN_NPROC. Why did you set it to 15? It's not
> unthinkable that future opclass frameworks will have different numbers
The BRIN_NPROC should be probably defined in brin.c since it's only used
for sizing local array variable in amvalidate and should be used to set
amsupport in the init function as well then.
> of support procs. For BRIN I'm thinking that we could add another
> support proc which validates the opclass definition using the specific
> framework; that way we will be able to check that the set of operators
> defined are correct, etc (something that the current approach cannot
> do).
As I said before in the thread I would prefer more granular approach to
validation - have amvalidateopclass in the struct for the current
functionality so that we can easily add more validators in the future.
There can still be one amvalidate function exposed on SQL level that
just calls all the amvalidate* functions that the am defines.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
