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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tommy Pavlicek <tommypav122(at)gmail(dot)com>
Cc: 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:47:48
Message-ID: 4077062.1687452468@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tommy Pavlicek <tommypav122(at)gmail(dot)com> writes:
> I've attached a couple of patches to allow ALTER OPERATOR to add
> commutators, negators, hashes and merges to operators that lack them.

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

> The first patch is create_op_fixes_v1.patch and it includes some
> refactoring in preparation for the ALTER OPERATOR changes and fixes a
> couple of minor bugs in CREATE OPERATOR:
> - prevents self negation when filling in/creating an existing shell operator
> - remove reference to sort operator in the self negation error message as
> the sort attribute appears to be deprecated in Postgres 8.3

Hmm, yeah, I bet nobody has looked at those edge cases in awhile.

> 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.)

regards, tom lane

[1] https://commitfest.postgresql.org/43/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2023-06-22 16:54:54 Re: [PATCH] Extend ALTER OPERATOR to support adding commutator, negator, hashes, and merges
Previous Message Andres Freund 2023-06-22 16:45:18 Re: vac_truncate_clog()'s bogus check leads to bogusness