Skip to content

Commit 29a20ff

Browse files
committed
Reject attempts to alter composite types used in indexes.
find_composite_type_dependencies() ignored indexes, which is a poor decision because an expression index could have a stored column of a composite (or other container) type even when the underlying table does not. Teach it to detect such cases and error out. We have to work a bit harder than for other relations because the pg_depend entry won't identify the specific index column of concern, but it's not much new code. This does not address bug #17872's original complaint that dropping a column in such a type might lead to violations of the uniqueness property that a unique index is supposed to ensure. That seems of much less concern to me because it won't lead to crashes. Per bug #17872 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/17872-d0fbb799dc3fd85d@postgresql.org
1 parent 1bbbe14 commit 29a20ff

File tree

3 files changed

+68
-10
lines changed

3 files changed

+68
-10
lines changed

src/backend/commands/tablecmds.c

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5962,6 +5962,7 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
59625962
{
59635963
Form_pg_depend pg_depend = (Form_pg_depend) GETSTRUCT(depTup);
59645964
Relation rel;
5965+
TupleDesc tupleDesc;
59655966
Form_pg_attribute att;
59665967

59675968
/* Check for directly dependent types */
@@ -5978,18 +5979,58 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
59785979
continue;
59795980
}
59805981

5981-
/* Else, ignore dependees that aren't user columns of relations */
5982-
/* (we assume system columns are never of interesting types) */
5983-
if (pg_depend->classid != RelationRelationId ||
5984-
pg_depend->objsubid <= 0)
5982+
/* Else, ignore dependees that aren't relations */
5983+
if (pg_depend->classid != RelationRelationId)
59855984
continue;
59865985

59875986
rel = relation_open(pg_depend->objid, AccessShareLock);
5988-
att = TupleDescAttr(rel->rd_att, pg_depend->objsubid - 1);
5987+
tupleDesc = RelationGetDescr(rel);
59895988

5990-
if (rel->rd_rel->relkind == RELKIND_RELATION ||
5991-
rel->rd_rel->relkind == RELKIND_MATVIEW ||
5992-
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
5989+
/*
5990+
* If objsubid identifies a specific column, refer to that in error
5991+
* messages. Otherwise, search to see if there's a user column of the
5992+
* type. (We assume system columns are never of interesting types.)
5993+
* The search is needed because an index containing an expression
5994+
* column of the target type will just be recorded as a whole-relation
5995+
* dependency. If we do not find a column of the type, the dependency
5996+
* must indicate that the type is transiently referenced in an index
5997+
* expression but not stored on disk, which we assume is OK, just as
5998+
* we do for references in views. (It could also be that the target
5999+
* type is embedded in some container type that is stored in an index
6000+
* column, but the previous recursion should catch such cases.)
6001+
*/
6002+
if (pg_depend->objsubid > 0 && pg_depend->objsubid <= tupleDesc->natts)
6003+
att = TupleDescAttr(tupleDesc, pg_depend->objsubid - 1);
6004+
else
6005+
{
6006+
att = NULL;
6007+
for (int attno = 1; attno <= tupleDesc->natts; attno++)
6008+
{
6009+
att = TupleDescAttr(tupleDesc, attno - 1);
6010+
if (att->atttypid == typeOid && !att->attisdropped)
6011+
break;
6012+
att = NULL;
6013+
}
6014+
if (att == NULL)
6015+
{
6016+
/* No such column, so assume OK */
6017+
relation_close(rel, AccessShareLock);
6018+
continue;
6019+
}
6020+
}
6021+
6022+
/*
6023+
* We definitely should reject if the relation has storage. If it's
6024+
* partitioned, then perhaps we don't have to reject: if there are
6025+
* partitions then we'll fail when we find one, else there is no
6026+
* stored data to worry about. However, it's possible that the type
6027+
* change would affect conclusions about whether the type is sortable
6028+
* or hashable and thus (if it's a partitioning column) break the
6029+
* partitioning rule. For now, reject for partitioned rels too.
6030+
*/
6031+
if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind) ||
6032+
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
6033+
rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
59936034
{
59946035
if (origTypeName)
59956036
ereport(ERROR,

src/test/regress/expected/alter_table.out

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3095,6 +3095,13 @@ CREATE TYPE test_type1 AS (a int, b text);
30953095
CREATE TABLE test_tbl1 (x int, y test_type1);
30963096
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
30973097
ERROR: cannot alter type "test_type1" because column "test_tbl1.y" uses it
3098+
DROP TABLE test_tbl1;
3099+
CREATE TABLE test_tbl1 (x int, y text);
3100+
CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1));
3101+
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
3102+
ERROR: cannot alter type "test_type1" because column "test_tbl1_idx.row" uses it
3103+
DROP TABLE test_tbl1;
3104+
DROP TYPE test_type1;
30983105
CREATE TYPE test_type2 AS (a int, b text);
30993106
CREATE TABLE test_tbl2 OF test_type2;
31003107
CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2);
@@ -3206,7 +3213,8 @@ Typed table of type: test_type2
32063213
c | text | | |
32073214
Inherits: test_tbl2
32083215

3209-
DROP TABLE test_tbl2_subclass;
3216+
DROP TABLE test_tbl2_subclass, test_tbl2;
3217+
DROP TYPE test_type2;
32103218
CREATE TYPE test_typex AS (a int, b text);
32113219
CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
32123220
ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails

src/test/regress/sql/alter_table.sql

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,14 @@ CREATE TYPE test_type1 AS (a int, b text);
19841984
CREATE TABLE test_tbl1 (x int, y test_type1);
19851985
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
19861986

1987+
DROP TABLE test_tbl1;
1988+
CREATE TABLE test_tbl1 (x int, y text);
1989+
CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1));
1990+
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
1991+
1992+
DROP TABLE test_tbl1;
1993+
DROP TYPE test_type1;
1994+
19871995
CREATE TYPE test_type2 AS (a int, b text);
19881996
CREATE TABLE test_tbl2 OF test_type2;
19891997
CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2);
@@ -2011,7 +2019,8 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
20112019
\d test_tbl2
20122020
\d test_tbl2_subclass
20132021

2014-
DROP TABLE test_tbl2_subclass;
2022+
DROP TABLE test_tbl2_subclass, test_tbl2;
2023+
DROP TYPE test_type2;
20152024

20162025
CREATE TYPE test_typex AS (a int, b text);
20172026
CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));

0 commit comments

Comments
 (0)