On 9/28/23 20:46, Tom Lane wrote:
Andrew Dunstan <and...@dunslane.net> writes:
I wonder if we could have a boolean flag in pg_enum, indicating that
setting an enum to that value was forbidden.
Yeah, but that still offers no coherent solution to the problem of
what happens if there's a table that already contains such a value.
It doesn't seem terribly useful to forbid new entries if you can't
get rid of old ones.
Admittedly, a DISABLE flag would at least offer a chance at a
race-condition-free scan to verify that no such values remain
in tables. But as somebody already mentioned upthread, that
wouldn't guarantee that the value doesn't appear in non-leaf
index pages. So basically you could never get rid of the pg_enum
row, short of a full dump and restore.
We went through all these points years ago when the enum feature
was first developed, as I recall. Nobody thought that the ability
to remove an enum value was worth the amount of complexity it'd
entail.
This issue comes up regularly (although far from often). Do we want to
put some comments right where would-be implementors would be sure to see it?
Attached is an example of what I mean. Documentation is intentionally
omitted.
--
Vik Fearing
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7d2032885e..f8d70cdaa0 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6404,6 +6404,29 @@ AlterEnumStmt:
n->skipIfNewValExists = false;
$$ = (Node *) n;
}
+ | ALTER TYPE_P any_name DROP VALUE_P Sconst
+ {
+ /*
+ * The following problems must be solved before this can be
+ * implemented:
+ *
+ * - There can be no data of this type using this value in
+ * any table
+ *
+ * - The value may not appear in any non-leaf page of a
+ * btree (and similar issues with other index types)
+ *
+ * - Concurrent sessions must not be able to insert this
+ * value while the preceding conditions are being checked
+ *
+ * - Possibly more...
+ */
+
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("dropping an enum value is not yet implemented"),
+ parser_errposition(@4)));
+ }
;
opt_if_not_exists: IF_P NOT EXISTS { $$ = true; }
diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out
index 01159688e5..105ae7a2f9 100644
--- a/src/test/regress/expected/enum.out
+++ b/src/test/regress/expected/enum.out
@@ -605,6 +605,11 @@ ERROR: "red" is not an existing enum label
-- check that renaming to an existent value fails
ALTER TYPE rainbow RENAME VALUE 'blue' TO 'green';
ERROR: enum label "green" already exists
+-- We still can't drop an enum value
+ALTER TYPE rainbow DROP VALUE 'green';
+ERROR: dropping an enum value is not yet implemented
+LINE 1: ALTER TYPE rainbow DROP VALUE 'green';
+ ^
--
-- check transactional behaviour of ALTER TYPE ... ADD VALUE
--
diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql
index 93171379f2..a98df40ed8 100644
--- a/src/test/regress/sql/enum.sql
+++ b/src/test/regress/sql/enum.sql
@@ -276,6 +276,9 @@ ALTER TYPE rainbow RENAME VALUE 'red' TO 'crimson';
-- check that renaming to an existent value fails
ALTER TYPE rainbow RENAME VALUE 'blue' TO 'green';
+-- We still can't drop an enum value
+ALTER TYPE rainbow DROP VALUE 'green';
+
--
-- check transactional behaviour of ALTER TYPE ... ADD VALUE
--
