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.
This commit is contained in:
Tom Lane 2012-09-22 18:35:22 -04:00
parent fcc1576687
commit 31510194cc
5 changed files with 33 additions and 25 deletions

View file

@ -109,15 +109,16 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD VALUE [ IF NOT
<term><literal>ADD VALUE [ IF NOT EXISTS ] [ BEFORE | AFTER ]</literal></term>
<listitem>
<para>
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 <literal>BEFORE</literal> or
<literal>AFTER</literal>, 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 <literal>BEFORE</literal>
or <literal>AFTER</literal> one of the existing values. Otherwise,
the new item is added at the end of the list of values.
</para>
<para>
If <literal>IF NOT EXISTS</literal> 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 <literal>IF NOT EXISTS</literal> 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.
</para>
</listitem>
</varlistentry>

View file

@ -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);

View file

@ -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;
/* ----------------------

View file

@ -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

View file

@ -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
--