From 50f6e73d9bc1e00ad3988faa811110a84af70aef Mon Sep 17 00:00:00 2001 From: Joseph Koshakow Date: Sat, 4 May 2024 10:12:37 -0400 Subject: [PATCH] Prevent name conflicts when dropping a column Previously, dropped columns were always renamed to "........pg.dropped.........". In the rare scenario that a column with that name already exists, the column drop would fail with an error about violating the unique constraint on "pg_attribute_relid_attnam_index". This commit fixes that issue by appending an int to dropped column name until we find a unique name. Since tables have a maximum of 16,000 columns and the max int is larger than 16,000, we are guaranteed to find a unique name. --- src/backend/catalog/heap.c | 16 +++++++++++++++- src/test/regress/expected/alter_table.out | 4 ++++ src/test/regress/sql/alter_table.sql | 5 +++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 922ba79ac2..852ed442e1 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1658,11 +1658,13 @@ RemoveAttributeById(Oid relid, AttrNumber attnum) Relation rel; Relation attr_rel; HeapTuple tuple; + HeapTuple drop_tuple_check; Form_pg_attribute attStruct; char newattname[NAMEDATALEN]; Datum valuesAtt[Natts_pg_attribute] = {0}; bool nullsAtt[Natts_pg_attribute] = {0}; bool replacesAtt[Natts_pg_attribute] = {0}; + int i; /* * Grab an exclusive lock on the target table, which we will NOT release @@ -1702,10 +1704,22 @@ RemoveAttributeById(Oid relid, AttrNumber attnum) attStruct->attgenerated = '\0'; /* - * Change the column name to something that isn't likely to conflict + * Change the column name to something that doesn't conflict */ snprintf(newattname, sizeof(newattname), "........pg.dropped.%d........", attnum); + Assert(PG_INT32_MAX > MaxHeapAttributeNumber); + drop_tuple_check = SearchSysCacheCopy2(ATTNAME, + ObjectIdGetDatum(relid), + PointerGetDatum(newattname)); + for (i = 0; HeapTupleIsValid(drop_tuple_check); i++) + { + snprintf(newattname, sizeof(newattname), + "........pg.dropped.%d.%d........", attnum, i); + drop_tuple_check = SearchSysCacheCopy2(ATTNAME, + ObjectIdGetDatum(relid), + PointerGetDatum(newattname)); + } namestrcpy(&(attStruct->attname), newattname); /* Clear the missing value */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 7666c76238..844ae55467 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1554,6 +1554,10 @@ insert into atacc1(id, value) values (null, 0); ERROR: null value in column "id" of relation "atacc1" violates not-null constraint DETAIL: Failing row contains (null, 0). drop table atacc1; +-- test dropping a column doesn't cause name conflicts +create table atacc1(a int, "........pg.dropped.1........" int); +alter table atacc1 drop column a; +drop table atacc1; -- test inheritance create table parent (a int, b int, c int); insert into parent values (1, 2, 3); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9df5a63bdf..d5d912a2e2 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1097,6 +1097,11 @@ insert into atacc1(value) values (100); insert into atacc1(id, value) values (null, 0); drop table atacc1; +-- test dropping a column doesn't cause name conflicts +create table atacc1(a int, "........pg.dropped.1........" int); +alter table atacc1 drop column a; +drop table atacc1; + -- test inheritance create table parent (a int, b int, c int); insert into parent values (1, 2, 3); -- 2.34.1