From a3c9d35ae168864bf7999b06e27dabe65a0915e9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 27 Mar 2023 15:04:02 -0400 Subject: [PATCH] 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 --- src/backend/commands/tablecmds.c | 56 +++++++++++++++++++---- src/test/regress/expected/alter_table.out | 10 +++- src/test/regress/sql/alter_table.sql | 11 ++++- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 9b0a0142d3..c510a01fd8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6508,6 +6508,7 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation, { Form_pg_depend pg_depend = (Form_pg_depend) GETSTRUCT(depTup); Relation rel; + TupleDesc tupleDesc; Form_pg_attribute att; /* Check for directly dependent types */ @@ -6524,18 +6525,57 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation, continue; } - /* Else, ignore dependees that aren't user columns of relations */ - /* (we assume system columns are never of interesting types) */ - if (pg_depend->classid != RelationRelationId || - pg_depend->objsubid <= 0) + /* Else, ignore dependees that aren't relations */ + if (pg_depend->classid != RelationRelationId) continue; rel = relation_open(pg_depend->objid, AccessShareLock); - att = TupleDescAttr(rel->rd_att, pg_depend->objsubid - 1); + tupleDesc = RelationGetDescr(rel); - if (rel->rd_rel->relkind == RELKIND_RELATION || - rel->rd_rel->relkind == RELKIND_MATVIEW || - rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + /* + * If objsubid identifies a specific column, refer to that in error + * messages. Otherwise, search to see if there's a user column of the + * type. (We assume system columns are never of interesting types.) + * The search is needed because an index containing an expression + * column of the target type will just be recorded as a whole-relation + * dependency. If we do not find a column of the type, the dependency + * must indicate that the type is transiently referenced in an index + * expression but not stored on disk, which we assume is OK, just as + * we do for references in views. (It could also be that the target + * type is embedded in some container type that is stored in an index + * column, but the previous recursion should catch such cases.) + */ + if (pg_depend->objsubid > 0 && pg_depend->objsubid <= tupleDesc->natts) + att = TupleDescAttr(tupleDesc, pg_depend->objsubid - 1); + else + { + att = NULL; + for (int attno = 1; attno <= tupleDesc->natts; attno++) + { + att = TupleDescAttr(tupleDesc, attno - 1); + if (att->atttypid == typeOid && !att->attisdropped) + break; + att = NULL; + } + if (att == NULL) + { + /* No such column, so assume OK */ + relation_close(rel, AccessShareLock); + continue; + } + } + + /* + * We definitely should reject if the relation has storage. If it's + * partitioned, then perhaps we don't have to reject: if there are + * partitions then we'll fail when we find one, else there is no + * stored data to worry about. However, it's possible that the type + * change would affect conclusions about whether the type is sortable + * or hashable and thus (if it's a partitioning column) break the + * partitioning rule. For now, reject for partitioned rels too. + */ + if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind) || + RELKIND_HAS_PARTITIONS(rel->rd_rel->relkind)) { if (origTypeName) ereport(ERROR, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 27b4d7dc96..3b708c7976 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3097,6 +3097,13 @@ CREATE TYPE test_type1 AS (a int, b text); CREATE TABLE test_tbl1 (x int, y test_type1); ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails ERROR: cannot alter type "test_type1" because column "test_tbl1.y" uses it +DROP TABLE test_tbl1; +CREATE TABLE test_tbl1 (x int, y text); +CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1)); +ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails +ERROR: cannot alter type "test_type1" because column "test_tbl1_idx.row" uses it +DROP TABLE test_tbl1; +DROP TYPE test_type1; CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2); @@ -3208,7 +3215,8 @@ Typed table of type: test_type2 c | text | | | Inherits: test_tbl2 -DROP TABLE test_tbl2_subclass; +DROP TABLE test_tbl2_subclass, test_tbl2; +DROP TYPE test_type2; CREATE TYPE test_typex AS (a int, b text); CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0)); ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 7dc9e3d632..58ea20ac3d 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1983,6 +1983,14 @@ CREATE TYPE test_type1 AS (a int, b text); CREATE TABLE test_tbl1 (x int, y test_type1); ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails +DROP TABLE test_tbl1; +CREATE TABLE test_tbl1 (x int, y text); +CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1)); +ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails + +DROP TABLE test_tbl1; +DROP TYPE test_type1; + CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2); @@ -2010,7 +2018,8 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE; \d test_tbl2 \d test_tbl2_subclass -DROP TABLE test_tbl2_subclass; +DROP TABLE test_tbl2_subclass, test_tbl2; +DROP TYPE test_type2; CREATE TYPE test_typex AS (a int, b text); CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));