Re: knngist - 0.8

Robert,

Are you sure postgres doesn't have limitation at all ? There are a lot ot them.
Of course, there are limitation and limitation and we should avoid limitations,
which will require incompatible changes in future in user's code, or which
prevent future improvements (getting rid limitation). We suggested
implementation of k-nn search, which has limitations, but in my opinion,
they are rather small and doesn't prevent in future to
write a descent patch, based on your five-key syscaches patches,
which will touch a lot of places in the pg source.

Who need boolean distance ? Ok, you insisted, we now support it.
Teodor wrote arguments (http://archives.postgresql.org/message-id/4C8E2590.6040802@sigaev.ru)
why two fields solution doesn't really helped.

You want "the points most distant from the bright center of the universe?",
sorry, for now. I think, this is a limitation we can live with for now.
It's k-nearest neighbourhood search, and not k-furthest outliers search.

You want documentation for review, I don't believe a reviewer can't review
without users documentation like CREATE/ALTER OPERATOR CLASS, etc.
Andrew Gierth was true,
that GiST documentation needed to be rewritten and he suggested to do that if
I understand him. I'd love to help, but I don't have any message from him.
If he changed his mind, I'll describe these changes.

We're not full-time pg-employers and it's not easy for us to follow
new cleaner-way. I think there is a risk, that there are will be lesser big
features introduced by non-pg employers in the future and eventually,
pg will be open-source database developed by pg-employers with some
amount cosmetic changes and fixes.

I suggest to find a sensible consensus on implementation, taking into
account Pareto Rule, and leave place for future improvement.

Oleg

On Tue, 14 Sep 2010, Robert Haas wrote:

> 2010/9/13 Teodor Sigaev <teodor(at)sigaev(dot)ru>:
>> [updated patch]
>
> I realize I'm repeating myself, but... this patch needs
> documentation. It's not optional.
>
> It seems to me that you need to do something about AMOPSTRATEGY.
> Hence the five-key syscaches patches I wrote that is sitting in queue.
> And also LookupOpClassInfo(). Am I confused?
>
> Is there a reason we're using a boolean 'amoporder' distinguish
> ordering operators from regular old operators? Tom and I were talking
> about some kind of an integer field, in case we need more categories
> later. You know, like 'amopcategory' or something like that. It
> could be just one byte, perhaps, but one bit seems unduly narrow. You
> could define constants OPCAT_QUAL and OPCAT_ORDER, or something like
> that.
>
> This error message seems like it ought to be complaining about the
> access method, not the index:
>
> + if (!pg_am->amcanorderbyop)
> + ereport(ERROR,
> +
> (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> + errmsg("index doesn't support
> ordering operations")));
>
> I sort of understand the reasons behind the following restriction, but
> is this truly the best we can do?
>
> + /*
> + * Currently, only descending and nulls last ordering
> is supported
> + */
> + if (!(pathkey->pk_strategy == BTLessStrategyNumber &&
> pathkey->pk_nulls_first == false))
> + return;
>
> What happens if we have an index strategy that can efficiently return
> the points most distant from the bright center of the universe?
>
> By the way:
>
> gistproc.c: In function ?gist_point_consistent?:
> gistproc.c:1023: error: ?distance? may be used uninitialized in this function
>
>

Regards,
Oleg
_____________________________________________________________
Oleg Bartunov, Research Scientist, Head of AstroNet (www.astronet.ru),
Sternberg Astronomical Institute, Moscow University, Russia
Internet: oleg(at)sai(dot)msu(dot)su, http://www.sai.msu.su/~megera/
phone: +007(495)939-16-83, +007(495)939-23-83