public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Allow alter type delete value in enum
4+ messages / 3 participants
[nested] [flat]
* [PATCH] Allow alter type delete value in enum
@ 2020-10-07 20:35 Maksim Kita <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Maksim Kita @ 2020-10-07 20:35 UTC (permalink / raw)
---
src/backend/catalog/pg_enum.c | 69 +++++++++++++++++++++++++++++++++
src/backend/commands/typecmds.c | 8 +++-
src/backend/parser/gram.y | 11 ++++++
src/backend/utils/adt/enum.c | 10 +++++
src/include/catalog/pg_enum.h | 2 +
5 files changed, 98 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index 27e4100a6f..907db93cf4 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -133,6 +133,7 @@ EnumValuesCreate(Oid enumTypeOid, List *vals)
values[Anum_pg_enum_enumsortorder - 1] = Float4GetDatum(elemno + 1);
namestrcpy(&enumlabel, lab);
values[Anum_pg_enum_enumlabel - 1] = NameGetDatum(&enumlabel);
+ values[Anum_pg_enum_isdropped - 1] = false;
tup = heap_form_tuple(RelationGetDescr(pg_enum), values, nulls);
@@ -485,6 +486,7 @@ restart:
values[Anum_pg_enum_enumsortorder - 1] = Float4GetDatum(newelemorder);
namestrcpy(&enumlabel, newVal);
values[Anum_pg_enum_enumlabel - 1] = NameGetDatum(&enumlabel);
+ values[Anum_pg_enum_isdropped - 1] = false;
enum_tup = heap_form_tuple(RelationGetDescr(pg_enum), values, nulls);
CatalogTupleInsert(pg_enum, enum_tup);
heap_freetuple(enum_tup);
@@ -583,6 +585,73 @@ 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)));
+
+ /* OK, make a writable copy of old tuple */
+ enum_tup = heap_copytuple(old_tup);
+ en = (Form_pg_enum) GETSTRUCT(enum_tup);
+
+ ReleaseCatCacheList(list);
+
+ /* Update the pg_enum entry */
+ en->isdropped = true;
+ CatalogTupleUpdate(pg_enum, &enum_tup->t_self, enum_tup);
+ heap_freetuple(enum_tup);
+
+ 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.c
index 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.y
index 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/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index 5ead794e34..397c02fa69 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -113,6 +113,7 @@ enum_in(PG_FUNCTION_ARGS)
Oid enumtypoid = PG_GETARG_OID(1);
Oid enumoid;
HeapTuple tup;
+ Form_pg_enum en;
/* must check length to prevent Assert failure within SearchSysCache */
if (strlen(name) >= NAMEDATALEN)
@@ -132,6 +133,15 @@ enum_in(PG_FUNCTION_ARGS)
format_type_be(enumtypoid),
name)));
+ en = (Form_pg_enum) GETSTRUCT(tup);
+
+ if (en->isdropped)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("enum value is dropped %s: \"%s\"",
+ format_type_be(enumtypoid),
+ name)));
+
/* check it's safe to use in SQL */
check_safe_enum_use(tup);
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h
index b28d441ba7..6a7b00ffbe 100644
--- a/src/include/catalog/pg_enum.h
+++ b/src/include/catalog/pg_enum.h
@@ -34,6 +34,7 @@ CATALOG(pg_enum,3501,EnumRelationId)
Oid enumtypid; /* OID of owning enum type */
float4 enumsortorder; /* sort position of this enum value */
NameData enumlabel; /* text representation of enum value */
+ bool isdropped; /* is enum value dropped */
} FormData_pg_enum;
/* ----------------
@@ -53,6 +54,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
--qDbXVdCdHGoSgWSk--
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
@ 2024-06-24 00:23 Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2024-06-24 00:23 UTC (permalink / raw)
To: Fabrízio de Royes Mello <[email protected]>; +Cc: Ranier Vilela <[email protected]>; pgsql-hackers
On Sun, Jun 23, 2024 at 09:08:47PM -0300, Fabrízio de Royes Mello wrote:
> Doesn’t “sizeof” solve the problem? It take in account the null-termination
> character.
The size of BackupState->name is fixed with MAXPGPATH + 1, so it would
be a better practice to use strlcpy() with sizeof(name) anyway?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
@ 2024-06-24 00:34 Ranier Vilela <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Ranier Vilela @ 2024-06-24 00:34 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Fabrízio de Royes Mello <[email protected]>; pgsql-hackers
Em dom., 23 de jun. de 2024 às 21:24, Michael Paquier <[email protected]>
escreveu:
> On Sun, Jun 23, 2024 at 09:08:47PM -0300, Fabrízio de Royes Mello wrote:
> > Doesn’t “sizeof” solve the problem? It take in account the
> null-termination
> > character.
>
> The size of BackupState->name is fixed with MAXPGPATH + 1, so it would
> be a better practice to use strlcpy() with sizeof(name) anyway?
>
It's not critical code, so I think it's ok to use strlen, even because the
result of strlen will already be available using modern compilers.
So, I think it's ok to use memcpy with strlen + 1.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c)
@ 2024-06-24 00:53 Michael Paquier <[email protected]>
parent: Ranier Vilela <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2024-06-24 00:53 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: Fabrízio de Royes Mello <[email protected]>; pgsql-hackers
On Sun, Jun 23, 2024 at 09:34:45PM -0300, Ranier Vilela wrote:
> It's not critical code, so I think it's ok to use strlen, even because the
> result of strlen will already be available using modern compilers.
>
> So, I think it's ok to use memcpy with strlen + 1.
It seems to me that there is a pretty good argument to just use
strlcpy() for the same reason as the one you cite: this is not a
performance-critical code, and that's just safer.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-06-24 00:53 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 20:35 [PATCH] Allow alter type delete value in enum Maksim Kita <[email protected]>
2024-06-24 00:23 Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c) Michael Paquier <[email protected]>
2024-06-24 00:34 ` Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c) Ranier Vilela <[email protected]>
2024-06-24 00:53 ` Re: Avoid incomplete copy string (src/backend/access/transam/xlog.c) Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox