Re: [HACKERS] [PATCH] Alter or rename enum value

Jim Nasby writes: > On 9/8/16 4:55 AM, Emre Hasegeli wrote: >> The main problem that has been discussed before was the indexes. One >> way is to tackle with it is to reindex all the tables after the >> operation. Currently we are doing it when the datatype of indexed

Re: [HACKERS] [PATCH] Alter or rename enum value

> Why not just disallow dropping a value that's still in use? That's certainly > what I would prefer to happen by default... Of course, we should disallow it. That problem is what to do next. We cannot just remove the value, because it might still be referenced from the inner nodes of the

Re: [HACKERS] [PATCH] Alter or rename enum value

On 9/8/16 4:55 AM, Emre Hasegeli wrote: The main problem that has been discussed before was the indexes. One way is to tackle with it is to reindex all the tables after the operation. Currently we are doing it when the datatype of indexed columns change. So it should be possible, but very

Re: [HACKERS] [PATCH] Alter or rename enum value

> Given that you are now familiar with the internals of how enums are > implemented would it be possible to continue the work and add the "ALTER > TYPE DROP VALUE " command? I was thinking to try developing it, but I would be more than happy to help by testing and reviewing, if someone else

Re: [HACKERS] [PATCH] Alter or rename enum value

On 7 September 2016 at 22:14, Tom Lane wrote: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > > Here is version 6 of the patch, which just adds RENAME VALUE with no IF > > [NOT] EXISTS, rebased onto current master (particularly the > > transactional

Re: [HACKERS] [PATCH] Alter or rename enum value

ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Here is version 6 of the patch, which just adds RENAME VALUE with no IF > [NOT] EXISTS, rebased onto current master (particularly the > transactional ADD VALUE patch). Pushed with some adjustments. The only thing that wasn't

Re: [HACKERS] [PATCH] Alter or rename enum value

Emre Hasegeli writes: >> Bottom line here is that I'd rather commit ALTER TYPE RENAME VALUE with >> no EXISTS features and then see it accrete those features together with >> other types of RENAME, when and if there's a will to make that happen. > > This sounds like a good

Re: [HACKERS] [PATCH] Alter or rename enum value

> Bottom line here is that I'd rather commit ALTER TYPE RENAME VALUE with > no EXISTS features and then see it accrete those features together with > other types of RENAME, when and if there's a will to make that happen. This sounds like a good conclusion to me. -- Sent via pgsql-hackers

Re: [HACKERS] [PATCH] Alter or rename enum value

Andrew Dunstan writes: > Are we also going to have an exists test for the original thing being > renamed? Exists tests on renames do strike me as somewhat cumbersome, to > say the least. I'm less opposed to that part, because it's at least got *some* precedent in existing

Re: [HACKERS] [PATCH] Alter or rename enum value

On 09/06/2016 02:30 PM, Tom Lane wrote: Robert Haas writes: On Mon, Sep 5, 2016 at 11:40 PM, Tom Lane wrote: ... And again, it's hard to get excited about having these options for RENAME VALUE when no one has felt a need for them yet in RENAME

Re: [HACKERS] [PATCH] Alter or rename enum value

Robert Haas writes: > On Mon, Sep 5, 2016 at 11:40 PM, Tom Lane wrote: >> ... And again, it's >> hard to get excited about having these options for RENAME VALUE when no >> one has felt a need for them yet in RENAME COLUMN. I'm especially dubious >>

Re: [HACKERS] [PATCH] Alter or rename enum value

On Mon, Sep 5, 2016 at 11:40 PM, Tom Lane wrote: > The opportunity cost here is potential user confusion. The only > closely parallel rename operation we have is ALTER TABLE RENAME COLUMN, > and that doesn't have a column-level IF EXISTS option; it has a > table-level IF

Re: [HACKERS] [PATCH] Alter or rename enum value

Emre Hasegeli writes: >> I started looking at this patch. I'm kind of unhappy with having *both* >> IF EXISTS and IF NOT EXISTS options on the statement, especially since >> the locations of those phrases in the syntax seem to have been chosen >> with a dartboard. This feels

Re: [HACKERS] [PATCH] Alter or rename enum value

> I started looking at this patch. I'm kind of unhappy with having *both* > IF EXISTS and IF NOT EXISTS options on the statement, especially since > the locations of those phrases in the syntax seem to have been chosen > with a dartboard. This feels way more confusing than it is useful. > Is

Re: [HACKERS] [PATCH] Alter or rename enum value

Emre Hasegeli writes: > Other than these, it looks good to me. I am marking it as Ready for > Committer. I started looking at this patch. I'm kind of unhappy with having *both* IF EXISTS and IF NOT EXISTS options on the statement, especially since the locations of those

Re: [HACKERS] [PATCH] Alter or rename enum value

> That would require changing it from an AlterEnumStmt to a RenameStmt > instead. Those look to me like they're for renaming SQL objects, > i.e. things addressed by identifiers, whereas enum labels are just > strings. Looking at the implementation of a few of the functions called > by

Re: [HACKERS] [PATCH] Alter or rename enum value

Emre Hasegeli writes: >> Here is v4, which changes the command from ALTER VALUE to RENAME VALUE, >> for consistency with RENAME ATTRIBUTE. > > It looks like we always use "ALTER ... RENAME ... old_name TO > new_name" syntax, so it is better that way. I have noticed that all >

Re: [HACKERS] [PATCH] Alter or rename enum value

Emre Hasegeli writes: >> +ReleaseCatCacheList(list); >> +heap_close(pg_enum, RowExclusiveLock); > Maybe we better release them before reporting error, too. I would > release the list after the loop, close the heap before ereport(). Transaction abort will

Re: [HACKERS] [PATCH] Alter or rename enum value

> Here is v4, which changes the command from ALTER VALUE to RENAME VALUE, > for consistency with RENAME ATTRIBUTE. It looks like we always use "ALTER ... RENAME ... old_name TO new_name" syntax, so it is better that way. I have noticed that all the other RENAMEs go through ExecRenameStmt(). We

Re: [HACKERS] [PATCH] Alter or rename enum value

> Emre, I noticed you modified the commitfest entry > (https://commitfest.postgresql.org/10/588/) to be for Andrew's > transactional enum addition patch instead, but didn't change the title. > I'll revert that as soon as it picks up this latest patch. Do you wish > to remain a reviewer for this

Re: [HACKERS] [PATCH] Alter or rename enum value

ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: > >> I was bored and thought "how hard could it be?", and a few hours' >> hacking later, I have something that seems to work. It doesn't do IF >> NOT EXISTS yet, and the error messaging

Re: [HACKERS] [PATCH] Alter or rename enum value

Marko Tiikkaja writes: > On 2016-03-27 19:30, Dagfinn Ilmari Mannsåker wrote: >> ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: >> >>> I was bored and thought "how hard could it be?", and a few hours' >>> hacking later, I have something that seems to work. It doesn't do IF

Re: [HACKERS] [PATCH] Alter or rename enum value

On 2016-03-27 19:30, Dagfinn Ilmari Mannsåker wrote: ilm...@ilmari.org (Dagfinn Ilmari Mannsåker) writes: I was bored and thought "how hard could it be?", and a few hours' hacking later, I have something that seems to work. It doesn't do IF NOT EXISTS yet, and the error messaging could do