| From: | Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> | 
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> | 
| Cc: | Tommy Pavlicek <tommypav122(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> | 
| Subject: | Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges | 
| Date: | 2023-06-22 16:54:54 | 
| Message-ID: | 87r0q3qvxd.fsf@wibble.ilmari.org | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Tommy Pavlicek <tommypav122(at)gmail(dot)com> 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.)
Since it only supports adding these operations if they don't already
exist, should it not be ALTER OPERATOR ADD <thing>, not SET <thing>?
That makes it natural to add an IF NOT EXISTS clause, like ALTER TABLE
ADD COLUMN has, to make it a no-op instead of an error.
> regards, tom lane
- ilmari
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2023-06-22 17:36:14 | Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges | 
| Previous Message | Tom Lane | 2023-06-22 16:47:48 | Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges |