Re: knngist - 0.8

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: knngist - 0.8
Date: 2010-11-12 17:24:13
Message-ID: 201011121724.oACHODB04788@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Robert, it is great you are taking this on. This is really a well-known
area of the code for you, but not so much for Teodor and Oleg, so I am
sure they appreciate your assistance.

---------------------------------------------------------------------------

Robert Haas wrote:
> On Sat, Oct 16, 2010 at 9:54 PM, Robert Haas <robertmhaas(at)gmail(dot)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

[ Attachment, skipping... ]

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2010-11-12 17:58:17 Re: locales and encodings Oh MY!
Previous Message Andrew Dunstan 2010-11-12 17:03:14 Re: Refactoring the Type System