Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges

Поиск
Список
Период
Сортировка
От Tommy Pavlicek
Тема Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges
Дата
Msg-id CAEhP-W_xGQVmiEJE_x0x5bbDSwsmaN1TQEawNadiMvQj2XK2Sg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
Tom Lane <tgl@sss.pgh.pa.us> writes:

> Please add this to the upcoming commitfest [1], to ensure we don't
> lose track of it.

I've added a single patch here: https://commitfest.postgresql.org/43/4389/

It wasn't obvious whether I should create a second commitfest entry
because I've included 2 patches so I've just done 1 to begin with. On
that note, is it preferred here to split patches of this size into
separate patches, and if so, additionally, separate threads?

Tom Lane <tgl@sss.pgh.pa.us> writes:

> > Additionally, I wasn't sure whether it was preferred to fail or succeed on
> > ALTERs that have no effect, such as adding hashes on an operator that
> > already allows them or disabling hashes on one that does not. I chose to
> > raise an error when this happens, on the thinking it was more explicit and
> > made the code simpler, even though the end result would be what the user
> > wanted.
>
> You could argue that both ways I guess.  We definitely need to raise error
> if the command tries to change an existing nondefault setting, since that
> might break things as per previous discussion.  But perhaps rejecting
> an attempt to set the existing setting is overly nanny-ish.  Personally
> I think I'd lean to "don't throw an error if we don't have to", but I'm
> not strongly set on that position.
>
> (Don't we have existing precedents that apply here?  I can't offhand
> think of any existing ALTER commands that would reject no-op requests,
> but maybe that's not a direct precedent.)

My initial thinking behind the error for a no-op was largely driven by
the existence of 'DROP.. IF EXISTS'. However, I did some ad hoc
testing on ALTER commands and it does seem that they mostly allow
no-ops. I did find that renaming an object to the same name will fail
due to the object already existing, but that seems to be more of a
coincidence than a design decision to me. Given this, I also lean
towards allowing the no-ops and will change it unless there are
objections.



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Dilip Kumar
Дата:
Сообщение: Re: Assert while autovacuum was executing
Следующее
От: Tom Lane
Дата:
Сообщение: Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges