From: Tom Lane Date: Sat, 22 Sep 2012 22:35:22 +0000 (-0400) Subject: Minor corrections for ALTER TYPE ADD VALUE IF NOT EXISTS patch. X-Git-Tag: REL9_3_BETA1~886 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=31510194cc9d87b355cb56e7d88c18c985d7a32a;p=postgresql Minor corrections for ALTER TYPE ADD VALUE IF NOT EXISTS patch. Produce a NOTICE when the label already exists, for consistency with other CREATE IF NOT EXISTS commands. Also, fix the code so it produces something more user-friendly than an index violation when the label already exists. This not incidentally enables making a regression test that the previous patch didn't make for fear of exposing an unpredictable OID in the results. Also some wordsmithing on the documentation. --- diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index 588887e1f9..99c9d50429 100644 --- a/doc/src/sgml/ref/alter_type.sgml +++ b/doc/src/sgml/ref/alter_type.sgml @@ -109,15 +109,16 @@ ALTER TYPE name ADD VALUE [ IF NOT ADD VALUE [ IF NOT EXISTS ] [ BEFORE | AFTER ] - This form adds a new value to an enum type. If the new value's place in - the enum's ordering is not specified using BEFORE or - AFTER, then the new item is placed at the end of the - list of values. + This form adds a new value to an enum type. The new value's place in + the enum's ordering can be specified as being BEFORE + or AFTER one of the existing values. Otherwise, + the new item is added at the end of the list of values. - If IF NOT EXISTS is used, it is not an error if the - type already contains the new value, and no action is taken. Otherwise, - an error will occur if the new value is already present. + If IF NOT EXISTS is specified, it is not an error if + the type already contains the new value: a notice is issued but no other + action is taken. Otherwise, an error will occur if the new value is + already present. diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c index f3161efb20..bed0f357bb 100644 --- a/src/backend/catalog/pg_enum.c +++ b/src/backend/catalog/pg_enum.c @@ -212,19 +212,30 @@ AddEnumLabel(Oid enumTypeOid, */ LockDatabaseObject(TypeRelationId, enumTypeOid, 0, ExclusiveLock); - /* Do the "IF NOT EXISTS" test if specified */ - if (skipIfExists) + /* + * Check if label is already in use. The unique index on pg_enum would + * catch this anyway, but we prefer a friendlier error message, and + * besides we need a check to support IF NOT EXISTS. + */ + enum_tup = SearchSysCache2(ENUMTYPOIDNAME, + ObjectIdGetDatum(enumTypeOid), + CStringGetDatum(newVal)); + if (HeapTupleIsValid(enum_tup)) { - HeapTuple tup; - - tup = SearchSysCache2(ENUMTYPOIDNAME, - ObjectIdGetDatum(enumTypeOid), - CStringGetDatum(newVal)); - if (HeapTupleIsValid(tup)) + ReleaseSysCache(enum_tup); + if (skipIfExists) { - ReleaseSysCache(tup); + ereport(NOTICE, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("enum label \"%s\" already exists, skipping", + newVal))); return; } + else + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("enum label \"%s\" already exists", + newVal))); } pg_enum = heap_open(EnumRelationId, RowExclusiveLock); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 98fe850c92..214d4f60e3 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2306,7 +2306,7 @@ typedef struct AlterEnumStmt char *newVal; /* new enum value's name */ char *newValNeighbor; /* neighboring enum value, if specified */ bool newValIsAfter; /* place new enum value after neighbor? */ - bool skipIfExists; /* ignore statement if label already exists */ + bool skipIfExists; /* no error if label already exists */ } AlterEnumStmt; /* ---------------------- diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out index a14097297a..ed729dddc3 100644 --- a/src/test/regress/expected/enum.out +++ b/src/test/regress/expected/enum.out @@ -97,11 +97,11 @@ ALTER TYPE planets ADD VALUE 'pluto' AFTER 'zeus'; ERROR: "zeus" is not an existing enum label -- if not exists tests -- existing value gives error --- We can't do this test because the error contains the --- offending Oid value, which is unpredictable. --- ALTER TYPE planets ADD VALUE 'mercury'; +ALTER TYPE planets ADD VALUE 'mercury'; +ERROR: enum label "mercury" already exists -- unless IF NOT EXISTS is specified ALTER TYPE planets ADD VALUE IF NOT EXISTS 'mercury'; +NOTICE: enum label "mercury" already exists, skipping -- should be neptune, not mercury SELECT enum_last(NULL::planets); enum_last diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql index db7bf44b40..130a723f69 100644 --- a/src/test/regress/sql/enum.sql +++ b/src/test/regress/sql/enum.sql @@ -57,10 +57,7 @@ ALTER TYPE planets ADD VALUE 'pluto' AFTER 'zeus'; -- if not exists tests -- existing value gives error - --- We can't do this test because the error contains the --- offending Oid value, which is unpredictable. --- ALTER TYPE planets ADD VALUE 'mercury'; +ALTER TYPE planets ADD VALUE 'mercury'; -- unless IF NOT EXISTS is specified ALTER TYPE planets ADD VALUE IF NOT EXISTS 'mercury'; @@ -73,7 +70,6 @@ ALTER TYPE planets ADD VALUE IF NOT EXISTS 'pluto'; -- should be pluto, i.e. the new value SELECT enum_last(NULL::planets); - -- -- Test inserting so many values that we have to renumber --