Re: Proposed new create command, CREATE OPERATOR CLASS

On Wed, 24 Oct 2001, Tom Lane wrote:

> Bill Studenmund <wrstuden(at)netbsd(dot)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 was writing a note saying that as this one came in. Yes, it's now a
TokenId, and I look for the text "needs_recheck".

> > 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.

Yep, I noticed that.

> > 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

So then funcname(type list) [, funcname(type list)] would be the way to
go?

> 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

Well, the requirement of binarity is something which is explicit in our
example documentation, and so that's why I used it.

> 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.

Do any of the access methods really support using non-binary operators?

> 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.

I can add support for skipping operators, if needed. A comma followed by a
comma would indicate a null name.

Oh gross. I just looked at contrib/intarray, and it defines two entries in
pg_amop for amopstrategy number 20. They do happen to be commutators of
each other. Look for the @@ and ~~ operators.

Wait a second, how can you do that? Doesn't that violate
pg_amop_opc_strategy_index ? It's supposed to make pairs of amopclaid and
amopstrategy be unique.

Confused....

> 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.

Which is why I didn't use it. :-)

If we can't make the other syntax work, then we can go with a DefineStmt
type syntax.

Take care,

Bill