Re: WIP: Enhanced ALTER OPERATOR

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Uriy Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP: Enhanced ALTER OPERATOR
Date: 2015-05-28 08:16:49
Message-ID: CAPpHfds9ghfawVRVCoLCXJWWbFheCuqQi1fvsLWJvySSbtJBsw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, May 27, 2015 at 9:28 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, May 18, 2015 at 10:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Uriy Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)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.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2015-05-28 08:18:18 Re: pg_upgrade resets timeline to 1
Previous Message Christoph Berg 2015-05-28 08:16:40 Re: pg_upgrade resets timeline to 1