Tommy Pavlicek <tommypav...@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