pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Tommy Pavlicek <tommypav122@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org, Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges
Date: Thu, 22 Jun 2023 17:54:54 +0100
Message-ID: <87r0q3qvxd.fsf@wibble.ilmari.org> (raw)
In-Reply-To: <4077062.1687452468@sss.pgh.pa.us>
References: <CAEhP-W-vGVzf4udhR5M8Bdv88UYnPrhoSkj3ieR3QNrsGQoqdg@mail.gmail.com>
	<4077062.1687452468@sss.pgh.pa.us>

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

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





view thread (25+ messages)  latest in thread

Message-ID: <87r0q3qvxd.fsf@wibble.ilmari.org>
Permalink:  ../87r0q3qvxd.fsf@wibble.ilmari.org/
Also on:    postgresql.org/message-id/87r0q3qvxd.fsf@wibble.ilmari.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: ilmari@ilmari.org, tgl@sss.pgh.pa.us, tommypav122@gmail.com, tomas.vondra@enterprisedb.com
  Subject: Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges
  In-Reply-To: <87r0q3qvxd.fsf@wibble.ilmari.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox