Hi all,
There's a rare edge case in `alter table` that can prevent users from
dropping a column as shown below
# create table atacc1(a int, "........pg.dropped.1........" int);
CREATE TABLE
# alter table atacc1 drop column a;
ERROR: duplicate key value violates unique constraint
"pg_attribute_relid_attnam_index"
DETAIL: Key (attrelid, attname)=(16407, ........pg.dropped.1........)
already exists.
It seems a bit silly and unlikely that anyone would ever find
themselves in this scenario, but it also seems easy enough to fix as
shown by the attached patch.
Does anyone think this is worth fixing? If so I can submit it to the
current commitfest.
Thanks,
Joe Koshakow
From 50f6e73d9bc1e00ad3988faa811110a84af70aef Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
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.<attnum>........". 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
