From: Maksim Kita <kitaetoya@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Subject: Allow deleting enum value
Date: Wed, 7 Oct 2020 17:26:56 +0300
Message-ID: <20201007142656.GA181015@yetti> (raw)
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
Attachments:0001-Allow-alter-type-delete-value-in-enum.patch (text/x-diff, 4.4K) download
From 2ae09254584340b7700e31680d752822a782be75 Mon Sep 17 00:00:00 2001
From: Maksim Kita <kitaetoya@gmail.com>
Date: Wed, 7 Oct 2020 17:18:01 +0300
Subject: [PATCH] Allow alter type delete value in enum
---
src/backend/catalog/pg_enum.c | 60 +++++++++++++++++++++++++++++++++
src/backend/commands/typecmds.c | 8 +++--
src/backend/parser/gram.y | 11 ++++++
src/include/catalog/pg_enum.h | 1 +
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.cindex 27e4100a6f..c547dfd22c 100644--- a/src/backend/catalog/pg_enum.c+++ b/src/backend/catalog/pg_enum.c@@ -583,6 +583,66 @@ RenameEnumLabel(Oid enumTypeOid,
table_close(pg_enum, RowExclusiveLock);
}
+extern void RemoveEnumLabel(Oid enumTypeOid, const char *val)+{+ Relation pg_enum;+ HeapTuple enum_tup;+ Form_pg_enum en;+ CatCList *list;+ int nelems;+ HeapTuple old_tup;+ int i;++ /* check length of new label is ok */+ if (strlen(val) > (NAMEDATALEN - 1))+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_NAME),+ errmsg("invalid enum label \"%s\"", val),+ errdetail("Labels must be %d characters or less.",+ NAMEDATALEN - 1)));++ /*+ * Acquire a lock on the enum type, which we won't release until commit.+ * This ensures that two backends aren't concurrently modifying the same+ * enum type. Since we are not changing the type's sort order, this is+ * probably not really necessary, but there seems no reason not to take+ * the lock to be sure.+ */+ LockDatabaseObject(TypeRelationId, enumTypeOid, 0, ExclusiveLock);++ pg_enum = table_open(EnumRelationId, RowExclusiveLock);++ /* Get the list of existing members of the enum */+ list = SearchSysCacheList1(ENUMTYPOIDNAME,+ ObjectIdGetDatum(enumTypeOid));+ nelems = list->n_members;++ /*+ * Check if the label is in use. (The unique index on pg_enum would catch that anyway, but we+ * prefer a friendlier error message.)+ */+ for (i = 0; i < nelems; i++)+ {+ enum_tup = &(list->members[i]->tuple);+ en = (Form_pg_enum) GETSTRUCT(enum_tup);+ if (strcmp(NameStr(en->enumlabel), val) == 0)+ {+ old_tup = enum_tup;+ break;+ }+ }+ if (!old_tup)+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("\"%s\" is not an existing enum label",+ val)));++ ReleaseCatCacheList(list);++ CatalogTupleDelete(pg_enum, &old_tup->t_self);++ table_close(pg_enum, RowExclusiveLock);+}
/*
* Test if the given enum value is on the blacklist
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.cindex 483bb65ddc..b9b3e18a94 100644--- a/src/backend/commands/typecmds.c+++ b/src/backend/commands/typecmds.c@@ -1232,13 +1232,17 @@ AlterEnum(AlterEnumStmt *stmt)
ReleaseSysCache(tup);
- if (stmt->oldVal)+ if (stmt->oldVal && stmt->newVal)
{
/* Rename an existing label */
RenameEnumLabel(enum_type_oid, stmt->oldVal, stmt->newVal);
}
- else+ else if (stmt->oldVal)
{
+ /* Delete an existing label */+ RemoveEnumLabel(enum_type_oid, stmt->oldVal);+ }+ else {
/* Add a new label */
AddEnumLabel(enum_type_oid, stmt->newVal,
stmt->newValNeighbor, stmt->newValIsAfter,
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.yindex 0d101d8171..82dd5147fa 100644--- a/src/backend/parser/gram.y+++ b/src/backend/parser/gram.y@@ -5792,6 +5792,17 @@ AlterEnumStmt:
n->skipIfNewValExists = false;
$$ = (Node *) n;
}
+ | ALTER TYPE_P any_name DELETE_P VALUE_P Sconst+ {+ AlterEnumStmt *n = makeNode(AlterEnumStmt);+ n->typeName = $3;+ n->oldVal = $6;+ n->newVal = NULL;+ n->newValNeighbor = NULL;+ n->newValIsAfter = false;+ n->skipIfNewValExists = false;+ $$ = (Node *) n;+ }
;
opt_if_not_exists: IF_P NOT EXISTS { $$ = true; }
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.hindex b28d441ba7..3dd81cd892 100644--- a/src/include/catalog/pg_enum.h+++ b/src/include/catalog/pg_enum.h@@ -53,6 +53,7 @@ extern void AddEnumLabel(Oid enumTypeOid, const char *newVal,
bool skipIfExists);
extern void RenameEnumLabel(Oid enumTypeOid,
const char *oldVal, const char *newVal);
+extern void RemoveEnumLabel(Oid enumTypeOid, const char *val);
extern bool EnumBlacklisted(Oid enum_id);
extern Size EstimateEnumBlacklistSpace(void);
extern void SerializeEnumBlacklist(void *space, Size size);
--
2.25.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: kitaetoya@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Allow deleting enum value
In-Reply-To: <20201007142656.GA181015@yetti>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox