Re: knngist - 0.8

On Sat, Oct 16, 2010 at 9:54 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Thinking about it that way, perhaps we could add an integer column
>> amop_whats_it_good_for that gets used as a bit field.  That wouldn't
>> require changing the index structure, although it might break some
>> other things.
>
> I gave this a shot (though I called it amoppurpose rather than
> amop_whats_it_good_for) and I think it's a reasonable way to proceed.
> Proof-of-concept patch attached.  This just adds the column (using the
> existing padding space), defines AMOP_SEARCH and AMOP_ORDER, and makes
> just about everything ignore anything not marked AMOP_SEARCH,
> attached.  This would obviously need some more hacking to pay
> attention to AMOP_ORDER where relevant, etc. and to create some actual
> syntax around it.  Currently CREATE OPERATOR CLASS / ALTER OPERATOR
> FAMILY have this bit:
>
> OPERATOR strategy_number ( op_type [ , op_type ] )
>
> knngist-0.9 implements this:
>
> [ORDER] OPERATOR strategy_number ( op_type [, op_type ] )
>
> ...but with the design proposed above that's not quite what we'd want,
> because amoppurpose is a bit field, so you could have one or both of
> the two possible purposes.  Perhaps:
>
> OPERATOR strategy_number ( op_type [ , op_type ] ) [ FOR { SEARCH |
> ORDER } [, ...] ]
>
> With the default being FOR SEARCH.

Slightly-more-fleshed out proof of concept patch attached, with actual
syntax, documentation, and pg_dump support added.  This might be
thought of as a subset of the builtin_knngist_core patch, without the
parts that make it actually do something useful (which is mostly
match_pathkey_to_index - which I'm still rather hoping to abstract in
some way via the access method interface, though I'm currently unsure
what the best way to do that is).

I notice that builtin_knngist_core checks whether the return type of
an ordering operator has a built-in btree opclass.  I'm not sure
whether we should bother checking that, because even if it's true I
don't think there's anything preventing it from becoming false later.
I think it's probably sufficient to just check this condition at plan
time and silently skip trying to build knn-type index paths if it's
not met.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company