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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9b0a0142d3b531f06d33608132398ba5733690bd..c510a01fd8dd9445dc1c6b7134c8c75252db2216 100644 (file)
--- 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 27b4d7dc96d9b82fa7a9353154679f7c8788fd68..3b708c7976bf5eb790129581dab98768e45a8498 100644 (file)
--- 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 7dc9e3d63295cfebe574d2a11a4a6bddc588c7d0..58ea20ac3dcb4cca5171620ecb07a55f65c51e2d 100644 (file)
--- 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));