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
-
Strip Windows newlines from extension script files manually.
- 6cfb3a337469 18.0 landed
-
Read extension script files in text not binary mode.
- 924e03917d6f 18.0 landed
-
Improve reporting of errors in extension script files.
- 774171c4f640 18.0 landed
-
Improve parser's reporting of statement start locations.
- 14e5680eee19 18.0 landed
-
Extend ALTER OPERATOR to allow setting more optimization attributes.
- 2b5154beab79 17.0 landed
-
Core support for "extensions", which are packages of SQL objects.
- d9572c4e3b47 9.1.0 cited