Hi,
I've briefly looked at the patch today. I think the idea is worthwhile,
but I found a couple of issues with the patch:
1) The index_selfuncs.h header is included in the wrong place, it should
be included before lsyscache.h (because 'i' < 'l').
2) I'm not sure it's a good idea to add dependency on a specific AM type
into indxpath.c. At the moment there are only two places, both referring
to BTREE_AM_OID, do we really hard-code another OID here?
I wonder if this could be generalized to another support proc in the
inde AM API, with just GIN implementing it.
3) selfuncs.c is hardly the right place for gin_get_optimizable_quals,
as it's only for functions computing selectivity estimates (and funcs
directly related to that). And the new function is not related to that
at all, so why not to define it in indxpath.c directly?
Of course, if it gets into the index AM API then this would disappear.
4) The gin_get_optimizable_quals is quite misleading. Firstly, it's not
very obvious what "optimizable" means in this context, but that's a
minor issue. The bigger issue is that it's a lie - when there are no
"optimizable" clauses (so all clauses would require full scan) the
function returns the original list, which is by definition completely
non-optimizable.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
