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.
Makes sense. I leave it amapi.h.
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.
Problem is that in BRIN, access method use only first 4 support procedures. However, operator class can define more and call them from first 4 support procedures. That allows operator classes to re-use same support procedures and build additional levels of abstractions. I've declared BRIN_MANDATORY_NPROCS, and brinvalidate checks only their presence. We could check BRIN opclass more precisely, by introducing new support procedure for validation. I think it's a subject of a separate patch since it would change interface of BRIN.
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.
I agree about staying with one SQL-visible function.
Other changes:
* Documentation reflects interface changes.
* IndexAmRoutine moved from CacheMemoryContext to indexcxt.
* Various minor formatting improvements.
* Error messages are corrected.
------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
