Be more clear when a new column name collides with a system column name.

We now use the same error message for ALTER TABLE .. ADD COLUMN or
ALTER TABLE .. RENAME COLUMN that we do for CREATE TABLE.  The old
message was accurate, but might be confusing to users not aware of our
system columns.

Vik Reykja, with some changes by me, and further proofreading by Tom Lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9172d999310e76b7153ea70b410e7c6a738864ef..07dc326cea7c4c034d4928e605b4fd8eedd43623 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -304,6 +304,7 @@ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recu
 static void ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
                ColumnDef *colDef, bool isOid,
                bool recurse, bool recursing, LOCKMODE lockmode);
+static void check_for_column_name_collision(Relation rel, const char *colname);
 static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
 static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
 static void ATPrepAddOids(List **wqueue, Relation rel, bool recurse,
@@ -2257,15 +2258,7 @@ renameatt_internal(Oid myrelid,
                        oldattname)));
 
    /* new name should not already exist */
-
-   /* this test is deliberately not attisdropped-aware */
-   if (SearchSysCacheExists2(ATTNAME,
-                             ObjectIdGetDatum(myrelid),
-                             PointerGetDatum(newattname)))
-       ereport(ERROR,
-               (errcode(ERRCODE_DUPLICATE_COLUMN),
-                errmsg("column \"%s\" of relation \"%s\" already exists",
-                     newattname, RelationGetRelationName(targetrelation))));
+   check_for_column_name_collision(targetrelation, newattname);
 
    /* apply the update */
    namestrcpy(&(attform->attname), newattname);
@@ -4310,17 +4303,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
        elog(ERROR, "cache lookup failed for relation %u", myrelid);
    relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
 
-   /*
-    * this test is deliberately not attisdropped-aware, since if one tries to
-    * add a column matching a dropped column name, it's gonna fail anyway.
-    */
-   if (SearchSysCacheExists2(ATTNAME,
-                             ObjectIdGetDatum(myrelid),
-                             PointerGetDatum(colDef->colname)))
-       ereport(ERROR,
-               (errcode(ERRCODE_DUPLICATE_COLUMN),
-                errmsg("column \"%s\" of relation \"%s\" already exists",
-                       colDef->colname, RelationGetRelationName(rel))));
+   /* new name should not already exist */
+   check_for_column_name_collision(rel, colDef->colname);
 
    /* Determine the new attribute's number */
    if (isOid)
@@ -4562,6 +4546,46 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
    }
 }
 
+/*
+ * If a new or renamed column will collide with the name of an existing
+ * column, error out.
+ */
+static void
+check_for_column_name_collision(Relation rel, const char *colname)
+{
+   HeapTuple   attTuple;
+       int         attnum;
+
+   /*
+    * this test is deliberately not attisdropped-aware, since if one tries to
+    * add a column matching a dropped column name, it's gonna fail anyway.
+    */
+   attTuple = SearchSysCache2(ATTNAME,
+                              ObjectIdGetDatum(RelationGetRelid(rel)),
+                              PointerGetDatum(colname));
+   if (!HeapTupleIsValid(attTuple))
+       return;
+
+       attnum = ((Form_pg_attribute) GETSTRUCT(attTuple))->attnum;
+   ReleaseSysCache(attTuple);
+
+   /*
+    * We throw a different error message for conflicts with system column
+    * names, since they are normally not shown and the user might otherwise
+    * be confused about the reason for the conflict.
+    */
+       if (attnum <= 0)
+       ereport(ERROR,
+               (errcode(ERRCODE_DUPLICATE_COLUMN),
+                errmsg("column name \"%s\" conflicts with a system column name",
+                       colname)));
+       else
+       ereport(ERROR,
+               (errcode(ERRCODE_DUPLICATE_COLUMN),
+                errmsg("column \"%s\" of relation \"%s\" already exists",
+                       colname, RelationGetRelationName(rel))));
+}
+
 /*
  * Install a column's dependency on its datatype.
  */

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e99254950426b6edea30ae125b1f5ba8e538b79e..4aba58c450361519a547d17d1b1d34c0b53b2be7 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -7,6 +7,8 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
 ERROR:  relation "tmp_wrong" does not exist
 COMMENT ON TABLE tmp IS 'table comment';
 COMMENT ON TABLE tmp IS NULL;
+ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
+ERROR:  column name "xmin" conflicts with a system column name
 ALTER TABLE tmp ADD COLUMN a int4 default 3;
 ALTER TABLE tmp ADD COLUMN b name;
 ALTER TABLE tmp ADD COLUMN c text;

diff --git a/src/test/regress/expected/errors.out b/src/test/regress/expected/errors.out
index 4a10c6ae8a0d3c969c6bd523ac93ca1e6e187288..fa0bd82819a89e1702b4cd26957dd9988420cfbc 100644 (file)
--- a/src/test/regress/expected/errors.out
+++ b/src/test/regress/expected/errors.out
@@ -109,7 +109,7 @@ alter table emp rename column salary to manager;
 ERROR:  column "manager" of relation "stud_emp" already exists
 -- conflict
 alter table emp rename column salary to oid;
-ERROR:  column "oid" of relation "stud_emp" already exists
+ERROR:  column name "oid" conflicts with a system column name
 --
 -- TRANSACTION STUFF
 -- not in a xact

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index d9bf08d6d5067ae2fd1321c103f52214a23e082f..d4e4c4958d186eb2a96699fac519db8ef8445554 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -9,6 +9,8 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
 COMMENT ON TABLE tmp IS 'table comment';
 COMMENT ON TABLE tmp IS NULL;
 
+ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
+
 ALTER TABLE tmp ADD COLUMN a int4 default 3;
 
 ALTER TABLE tmp ADD COLUMN b name;