Tommy Pavlicek <tommypav122@gmail.com> writes:
> I've attached a new version of the ALTER OPERATOR patch that allows
> no-ops. It should be ready to review now.
I got around to looking through this finally (sorry about the delay).
I'm mostly on board with the functionality, with the exception that
I don't see why we should allow ALTER OPERATOR to cause new shell
operators to be created. The argument for allowing that in CREATE
OPERATOR was mainly to allow a linked pair of operators to be created
without a lot of complexity (specifically, being careful to specify
the commutator or negator linkage only in the second CREATE, which
is a rule that requires another exception for a self-commutator).
However, if you're using ALTER OPERATOR then you might as well create
both operators first and then link them with an ALTER command.
In fact, I don't really see a use-case where the operators wouldn't
both exist; isn't this feature mainly to allow retrospective
correction of omitted linkages? So I think allowing ALTER to create a
second operator is more likely to allow mistakes to sneak by than to
do anything useful --- and they will be mistakes you can't correct
except by starting over. I'd even question whether we want to let
ALTER establish a linkage to an existing shell operator, rather than
insisting you turn it into a valid operator via CREATE first.
If we implement it with that restriction then I don't think the
refactorization done in 0001 is correct, or at least not ideal.
(In any case, it seems like a bad idea that the command reference
pages make no mention of this stuff about shell operators. It's
explained in 38.15. Operator Optimization Information, but it'd
be worth at least alluding to that section here. Or maybe we
should move that info to CREATE OPERATOR?)
More generally, you muttered something about 0001 fixing some
existing bugs, but if so I can't see those trees for the forest of
refactorization. I'd suggest splitting any bug fixes apart from
the pure-refactorization step.
regards, tom lane