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 588887e1f9b89bc0cb8f73f9b5dcb8a66b17bfb9..99c9d50429d4898e2e5716ca229fcd56154c2f8e 100644 (file)
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -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>

diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index f3161efb20b94ccb7b8f2707caf052b53f05c67c..bed0f357bb6c91626b322d231c8dbb5dc8e4ed70 100644 (file)
--- 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 98fe850c927c03d23dae36949dda2bf2a70166db..214d4f60e323c819a5e5c3c7e251f0ef67fa8f5e 100644 (file)
--- 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 a14097297a10e33392cac302f355af81c41ad00f..ed729dddc3f3fdfece86934d48c6645e480cef4a 100644 (file)
--- 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 db7bf44b408ab5970a4ede560229c09774321a77..130a723f698afce670bb2b1abd12b050524457d2 100644 (file)
--- 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
 --