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

Tommy Pavlicek <tommypav122@gmail.com>

From: Tommy Pavlicek <tommypav122@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>, pgsql-hackers@lists.postgresql.org, Tomas Vondra <tomas.vondra@enterprisedb.com>
Date: 2023-06-23T10:34:38Z
Lists: 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.



Commits

  1. Strip Windows newlines from extension script files manually.

  2. Read extension script files in text not binary mode.

  3. Improve reporting of errors in extension script files.

  4. Improve parser's reporting of statement start locations.

  5. Extend ALTER OPERATOR to allow setting more optimization attributes.

  6. Core support for "extensions", which are packages of SQL objects.