Re: Proposed new create command, CREATE OPERATOR CLASS

Bill Studenmund <wrstuden@netbsd.org> writes:
> I'd like to propose a new command, CREATE OPERATOR CLASS.

Seems like a good idea.

> operator spec is either an operator or an operator followed by the keyword
> "REPEATABLE". The presence of "REPEATABLE" indicates that amopreqcheck
> should be set to true for this operator.

This is bogus, since REPEATABLE is a very poor description of the
meaning of amopreqcheck; to the extent that it matches the meaning
at all, it's backwards.  Don't pick a keyword for this solely on the
basis of what you can find that's already reserved by SQL99.

Given the restricted syntax, the keyword could be a TokenId anyway,
so it's not really reserved; accordingly there's no need to limit
ourselves to what SQL99 says we can reserve.

Perhaps use "RECHECK"?  That would fit the field more closely...

> I agree that I think it is rare that anything will set "REPEATABLE", but
> the point of this effort is to keep folks from mucking around with the
> system tables manually, so we should support making any reasonable entry
> in pg_amop.

Then you'd better add support for specifying an opckeytype, too.  BTW
these things are not all that rare; there are examples right now in
contrib.

> CREATE OPERATOR CLASS complex_abs_ops DEFAULT FOR TYPE complex USING
> btree with ||<, ||<=, ||=, ||>=, ||> and complex_abs_cmp;

This syntax is obviously insufficient to identify the procedures, since
it doesn't show argument lists (and we do allow overloading).  Less
obviously, it's not sufficient to identify the operators either.  I
think you're implicitly assuming that only binary operators on the
specified type will ever be members of index opclasses.  That does not
seem like a good assumption to wire into the syntax.  Perhaps borrow
the syntax used for DROP OPERATOR, which is ugly but not ambiguous:
operator (type, type)operator (type, NONE)operator (NONE, type)

We could allow an operator without any parenthesized args to imply a
binary op on the specified type, which would certainly be the most
common case.

BTW, is there any need to support filling nonconsecutive amopstrategy or
amprocnum slots?  This syntax can't do that.  GiST seems to have a
pretty loose idea of what set of strategy numbers you can have, so
there might possibly be a future need for that.

Also, it might be better to use a syntax in the style of CREATE
OPERATOR, with a list of param = value notations, because that's
more easily extensible if we change the opclass stuff again.
CREATE OPERATOR CLASS classname (    basetype = complex,    default,    operator1 = ||< ,    ...    proc1 =
complex_abs_cmp);
 

However, specifying the proc arglists in this style would be awfully
tedious :-(.  I can't think of anything better than
    proc1arg1 = complex,    proc1arg2 = complex,    ...

which is mighty ugly.
        regards, tom lane