Обсуждение: Allow deleting enum value

Hi,

There is a question related to TODO task with name "Allow deleting enumerated values from an existing enumerated data
type".
I made simple changes in parser and implement RemoveEnumLabel function, but as I understand if enum value that we want
to
 
delete is in use by some tables, we have to prevent deletion to avoid inconsistency.

Could you please provide some references where similar functionality was implemented  ? Thanks.

Attached basic patch without handling dependencies. 

Example of problem that I mention using basic implementation:

postgres=# CREATE TYPE enum_test as ENUM ('1', '2', '3');
CREATE TYPE
postgres=# CREATE TYPE test_enum as ENUM ('1', '2', '3');
CREATE TYPE
postgres=# CREATE TABLE test_table (value test_enum);
CREATE TABLE
postgres=# INSERT INTO test_table VALUES ('1'), ('2');
INSERT 0 2
postgres=# ALTER TYPE test_enum DELETE VALUE '2';
ALTER TYPE
postgres=# SELECT enum_range(NULL::test_enum);
 enum_range
------------
 {1,3}
(1 row)

postgres=# SELECT * FROM test_table;
ERROR:  invalid internal value for enum: 16396

Best regards,
Maksim Kita

Maksim Kita <kitaetoya@gmail.com> writes:
> There is a question related to TODO task with name "Allow deleting enumerated values from an existing enumerated data
type".
> I made simple changes in parser and implement RemoveEnumLabel function, but as I understand if enum value that we
wantto  
> delete is in use by some tables, we have to prevent deletion to avoid inconsistency.

It's a lot worse than that.  Even if you could ensure that the value is no
longer present in any tables (which you cannot really, because of race
conditions), that is not sufficient to ensure that it's not present in any
indexes.  For example, if a specific value has managed to work its way up
into the upper levels of a btree index, it's basically never going to
disappear short of a full REINDEX.  So we have to keep around sufficient
information to allow it to be compared correctly.

That TODO item should either be removed or marked with a warning stating
that it's next door to impossible.  (Unfortunately, a lot of our TODO
items are like that ... there's usually a good reason why they're not
done already.)

            regards, tom lane

I wrote:
> That TODO item should either be removed or marked with a warning stating
> that it's next door to impossible.  (Unfortunately, a lot of our TODO
> items are like that ... there's usually a good reason why they're not
> done already.)

Actually ... I'm not sure if this idea has been discussed before, but
what if we redefined what "delete" means?  We could add an "isdropped"
column to pg_enum, and have DROP do nothing but set that flag, and
modify enum_in to reject such values.  Then comparisons still work,
and whether there are remaining instances of the value is the
user's problem not ours.

pg_dump and pg_upgrade would need to jump through some hoops to
deal with this, but it's not any harder than a lot of other
weird cases they deal with.

            regards, tom lane

I like the idea, during prototype I added additional column and modified
enum_in method. But enum_in is called in contexts that can be important
for user (like comparisons).

Example:

postgres=# CREATE TYPE test_enum AS enum ('1', '2', '3');
CREATE TYPE

postgres=# CREATE TABLE test_table ( value test_enum );
postgres=# INSERT INTO test_table VALUES ('1'), ('2'), ('3');
INSERT 0 3

postgres=# ALTER TYPE test_enum DELETE VALUE '2';
ALTER TYPE

postgres=# INSERT INTO test_table VALUES ('2');
ERROR:  enum value is dropped test_enum: "2"
LINE 1: INSERT INTO test_table VALUES ('2');

postgres=# SELECT * FROM test_table WHERE value = '2';
ERROR:  enum value is dropped test_enum: "2"
LINE 1: SELECT * FROM test_table WHERE value = '2';

postgres=# UPDATE test_table SET value = '3' WHERE value = '2';
ERROR:  enum value is dropped test_enum: "2"
LINE 1: UPDATE test_table SET value = '3' WHERE value = '2';

Probably we need to make more specific change for enum type to prevent
using of dropped column in context of insert or update (where we
creating dropped enum value), but not in others.

Is that possible ? What places should I look into ? Thanks.

Best regards,
Maksim Kita

On Wed, Oct 07, 2020 at 11:47:07PM +0300, Maksim Kita wrote:
> I like the idea, during prototype I added additional column and modified
> enum_in method. But enum_in is called in contexts that can be important
> for user (like comparisons).
...
> postgres=# ALTER TYPE test_enum DELETE VALUE '2';
> ALTER TYPE

I think it should be called "DROP VALUE"

> postgres=# SELECT * FROM test_table WHERE value = '2';
> ERROR:  enum value is dropped test_enum: "2"
> LINE 1: SELECT * FROM test_table WHERE value = '2';

This fails if the value was never added, so why wouldn't it also fail if the
value is added and then removed ?

Maybe you'd want to rename old enum names to something "unlikely", like what we
do for dropped attributes in RemoveAttributeById.

How do you want to handle "adding a value back" ?

I think you should determine/propose/discuss the desired behaviors before
implementing it.

I think you'd also need to update these:
src/bin/pg_dump/pg_dump.c
src/bin/psql/describe.c
src/bin/psql/tab-complete.c
doc/src/sgml/catalogs.sgml
src/test/regress/sql/enum.sql
src/test/regress/expected/enum.out

-- 
Justin