2010/7/22 Teodor Sigaev <teodor(at)sigaev(dot)ru>:
> * pg_amop has boolean column amoporder which points to clause's usage of
> * Syntax of CREATE OPERATOR CLASS
> CREATE OPERATOR CLASS ...
> [ORDER] OPERATOR ....
> ORDER OPERATOR is marked with pg_amop.amoporder = true
> * Bool-returning operator could not be used as ORDER OPERATOR, but type of
> returning value still should have a default Btree operator class.
> * Added flag SK_ORDER to SkanKey flag to indicate order operation, so access
> methods (only GiST right now) should check this flag (in previous versions
> patch GiST checked returned value of operator's function)
AFAICS, these patches include no documentation. That's pretty much a
fatal flaw for a feature of this magnitude. At an absolute minimum,
you need to update the system catalog documentation and the
documentation on CREATE / ALTER OPERATOR CLASS. There might be some
other places that need to be touched, also.
+ if (opform->oprresult == BOOLOID)
+ errmsg("index ordering
operators must not return boolean")));
My first thought was that this code was there to prevent people from
doing the wrong thing by accident. But I have a niggling feeling that
you're actually relying on this for the correctness of the system. I
hope I'm wrong, because I don't think that would be a very good idea.
The GIST code code use more comments; and perhaps the names of some of
the functions and structures could be chosen to be more descriptive.
I think that what used to be called GISTSearchStack has apparently
been replaced with DataPointer; it's not obvious to me that it's good
to change the name, but if it is I don't think DataPointer is a good
choice. gistindex_keytest has been replaced (sort of) by
processIndexTuple, which again seems more generic than what it
Minor nit: the word "shoould" is mis-spelled.
The Enterprise Postgres Company
In response to
pgsql-hackers by date
|Next:||From: David Christensen||Date: 2010-09-07 01:03:09|
|Subject: Re: WIP: Triggers on VIEWs|
|Previous:||From: Robert Haas||Date: 2010-09-06 23:49:39|
|Subject: Re: Synchronization levels in SR|