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

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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to