On Mon, May 18, 2015 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Uriy Zhuravlev <u.zhuravlev@postgrespro.ru> writes: >> I have attached a patch that extends ALTER OPERATOR to support COMMUTATOR, >> NEGATOR, RESTRICT and JOIN. > > There are fairly significant reasons why we have not done this, based > on the difficulty of updating existing cached plans that might have > incidentally depended on those operator properties during creation. > Perhaps it's all right to simply ignore such concerns, but I would like > to see a defense of why.
I don't think there's a direct problem with cached plans, because it looks like plancache.c blows away the entire plan cache for any change to pg_operator. OperatorUpd() can already update oprcom and oprnegate, which seems to stand for the proposition that it's OK to set those from InvalidOid to something else. But that doesn't prove that other kinds of changes are safe.
A search of other places where oprcom is used in the code led me to ComputeIndexAttrs(). If an operator whose commutator is itself were changed so that the commutator was anything else, then we'd end up with a broken exclusion constraint. That's a problem. targetIsInSortList is run during parse analysis, and that too tests whether sortop == get_commutator(scl->sortop). Those decisions wouldn't get reevaluated if the truth of that expression changed after the fact, which I suspect is also a problem.
Could we address both this problems by denying changing existing commutators and negator? ISTM that setting absent commutator and negator is quite enough for ALTER OPERATOR. User extensions could need setting of commutator and negator because they could add new operators which don't exist before. But it's rather uncommon to unset or change commutator or negator.
