Neil Conway <neilc@samurai.com> writes:
> This patch moves GiST implementation details from gist.h into a new
> header file, gist_private.h.
Sounds reasonable. The other index AMs could possibly benefit from the
same thing --- in particular I believe nbtree.h is included by quite a
lot of places that really only need to know btree strategy and support
procedure numbers.
One objection: I think the GiST amproc numbers (GIST_CONSISTENT_PROC
and friends) *are* part of the API and should be in the public header,
even if they happen not to be used by any C code at the moment. They
are certainly well known at the SQL level, and the btree precedent
suggests people will want them at the C level too.
> I noticed that GISTNStrategies is defined, but never used; instead there
> is a literal "100" in include/catalog/pg_am.h. Does anyone see a reason
> to keep GISTNStrategies around? Alternatively, should pg_am.h include
> gist.h and reference GISTNStrategies instead of using "100"?
GISTNStrategies seems inherently bogus, since there's no essential limit
on the number of strategies in a gist index. I'd get rid of it.
The "100" in pg_am.h is pretty nasty too, because it is on the one hand
theoretically insufficient and on the other hand in practice way too
much. (This at least leads to wasted space in relcache entries for gist
indexes, and probably defeats error checks to some extent as well.)
It might be interesting someday to think about a way to let this number
be specified per-opclass instead of per-AM, for those AMs where such a
thing makes sense (only GiST at the moment). I'm not terribly excited
about it though... for now I'm willing to live with "100". Anyone who
needs more can poke their local copy of pg_am.
regards, tom lane