Skip to content

Commit 2d1371d

Browse files
robertmhaasrobertmhaas
committed
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
1 parent d4bad4e commit 2d1371d

File tree

4 files changed

+49
-21
lines changed

4 files changed

+49
-21
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recu
304304
static void ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
305305
ColumnDef *colDef, bool isOid,
306306
bool recurse, bool recursing, LOCKMODE lockmode);
307+
static void check_for_column_name_collision(Relation rel, const char *colname);
307308
static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
308309
static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
309310
static void ATPrepAddOids(List **wqueue, Relation rel, bool recurse,
@@ -2257,15 +2258,7 @@ renameatt_internal(Oid myrelid,
22572258
oldattname)));
22582259

22592260
/* new name should not already exist */
2260-
2261-
/* this test is deliberately not attisdropped-aware */
2262-
if (SearchSysCacheExists2(ATTNAME,
2263-
ObjectIdGetDatum(myrelid),
2264-
PointerGetDatum(newattname)))
2265-
ereport(ERROR,
2266-
(errcode(ERRCODE_DUPLICATE_COLUMN),
2267-
errmsg("column \"%s\" of relation \"%s\" already exists",
2268-
newattname, RelationGetRelationName(targetrelation))));
2261+
check_for_column_name_collision(targetrelation, newattname);
22692262

22702263
/* apply the update */
22712264
namestrcpy(&(attform->attname), newattname);
@@ -4310,17 +4303,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
43104303
elog(ERROR, "cache lookup failed for relation %u", myrelid);
43114304
relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
43124305

4313-
/*
4314-
* this test is deliberately not attisdropped-aware, since if one tries to
4315-
* add a column matching a dropped column name, it's gonna fail anyway.
4316-
*/
4317-
if (SearchSysCacheExists2(ATTNAME,
4318-
ObjectIdGetDatum(myrelid),
4319-
PointerGetDatum(colDef->colname)))
4320-
ereport(ERROR,
4321-
(errcode(ERRCODE_DUPLICATE_COLUMN),
4322-
errmsg("column \"%s\" of relation \"%s\" already exists",
4323-
colDef->colname, RelationGetRelationName(rel))));
4306+
/* new name should not already exist */
4307+
check_for_column_name_collision(rel, colDef->colname);
43244308

43254309
/* Determine the new attribute's number */
43264310
if (isOid)
@@ -4562,6 +4546,46 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
45624546
}
45634547
}
45644548

4549+
/*
4550+
* If a new or renamed column will collide with the name of an existing
4551+
* column, error out.
4552+
*/
4553+
static void
4554+
check_for_column_name_collision(Relation rel, const char *colname)
4555+
{
4556+
HeapTuple attTuple;
4557+
int attnum;
4558+
4559+
/*
4560+
* this test is deliberately not attisdropped-aware, since if one tries to
4561+
* add a column matching a dropped column name, it's gonna fail anyway.
4562+
*/
4563+
attTuple = SearchSysCache2(ATTNAME,
4564+
ObjectIdGetDatum(RelationGetRelid(rel)),
4565+
PointerGetDatum(colname));
4566+
if (!HeapTupleIsValid(attTuple))
4567+
return;
4568+
4569+
attnum = ((Form_pg_attribute) GETSTRUCT(attTuple))->attnum;
4570+
ReleaseSysCache(attTuple);
4571+
4572+
/*
4573+
* We throw a different error message for conflicts with system column
4574+
* names, since they are normally not shown and the user might otherwise
4575+
* be confused about the reason for the conflict.
4576+
*/
4577+
if (attnum <= 0)
4578+
ereport(ERROR,
4579+
(errcode(ERRCODE_DUPLICATE_COLUMN),
4580+
errmsg("column name \"%s\" conflicts with a system column name",
4581+
colname)));
4582+
else
4583+
ereport(ERROR,
4584+
(errcode(ERRCODE_DUPLICATE_COLUMN),
4585+
errmsg("column \"%s\" of relation \"%s\" already exists",
4586+
colname, RelationGetRelationName(rel))));
4587+
}
4588+
45654589
/*
45664590
* Install a column's dependency on its datatype.
45674591
*/

‎src/test/regress/expected/alter_table.out

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
77
ERROR: relation "tmp_wrong" does not exist
88
COMMENT ON TABLE tmp IS 'table comment';
99
COMMENT ON TABLE tmp IS NULL;
10+
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
11+
ERROR: column name "xmin" conflicts with a system column name
1012
ALTER TABLE tmp ADD COLUMN a int4 default 3;
1113
ALTER TABLE tmp ADD COLUMN b name;
1214
ALTER TABLE tmp ADD COLUMN c text;

‎src/test/regress/expected/errors.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ alter table emp rename column salary to manager;
109109
ERROR: column "manager" of relation "stud_emp" already exists
110110
-- conflict
111111
alter table emp rename column salary to oid;
112-
ERROR: column "oid" of relation "stud_emp" already exists
112+
ERROR: column name "oid" conflicts with a system column name
113113
--
114114
-- TRANSACTION STUFF
115115
-- not in a xact

‎src/test/regress/sql/alter_table.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
99
COMMENT ON TABLE tmp IS 'table comment';
1010
COMMENT ON TABLE tmp IS NULL;
1111

12+
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
13+
1214
ALTER TABLE tmp ADD COLUMN a int4 default 3;
1315

1416
ALTER TABLE tmp ADD COLUMN b name;

0 commit comments

Comments
 (0)