aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2023-03-27 15:04:02 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2023-03-27 15:04:02 -0400
commit29a20ff0617ab60acbb8d71b6b59f9035c86a4f8 (patch)
tree8c32775a51249580dac9061912f10bbb35493872
parent1bbbe14607c3590c977dc0aeddfdc10680f6bd39 (diff)
downloadpostgresql-29a20ff0617ab60acbb8d71b6b59f9035c86a4f8.tar.gz
postgresql-29a20ff0617ab60acbb8d71b6b59f9035c86a4f8.zip
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
-rw-r--r--src/backend/commands/tablecmds.c57
-rw-r--r--src/test/regress/expected/alter_table.out10
-rw-r--r--src/test/regress/sql/alter_table.sql11
3 files changed, 68 insertions, 10 deletions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5d0dcfa3e6e..5c596499d1b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5962,6 +5962,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 */
@@ -5978,18 +5979,58 @@ 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) ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
+ rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
{
if (origTypeName)
ereport(ERROR,
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 879a5c93a0d..47d26ef632c 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3095,6 +3095,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);
@@ -3206,7 +3213,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 c3b1a0cde6c..53e62c5ff9b 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1984,6 +1984,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);
@@ -2011,7 +2019,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));