8000 Reject attempts to alter composite types used in indexes. · postgres/postgres@78838bc · GitHub
[go: up one dir, main page]
Skip to content

Commit 78838bc

Browse files
tglsfdctglsfdc
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 ae320fc commit 78838bc

File tree

3 files changed

+70
-8
lines changed

3 files changed

+70
-8
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5360,6 +5360,7 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
53605360
{
53615361
Form_pg_depend pg_depend = (Form_pg_depend) GETSTRUCT(depTup);
53625362
Relation rel;
5363+
TupleDesc tupleDesc;
53635364
Form_pg_attribute att;
53645365

53655366
/* Check for directly dependent types */
@@ -5376,18 +5377,62 @@ find_composite_type_dependencies(Oid typeOid, Relation origRelation,
53765377
continue;
53775378
}
53785379

5379-
/* Else, ignore dependees that aren't user columns of relations */
5380-
/* (we assume system columns are never of interesting types) */
5381-
if (pg_depend->classid != RelationRelationId ||
5382-
pg_depend->objsubid <= 0)
5380+
/* Else, ignore dependees that aren't relations */
5381+
if (pg_depend->classid != RelationRelationId)
53835382
continue;
53845383

53855384
rel = relation_open(pg_depend->objid, AccessShareLock);
5386-
att = TupleDescAttr(rel->rd_att, pg_depend->objsubid - 1);
5385+
tupleDesc = RelationGetDescr(rel);
5386+
5387+
/*
5388+
* If objsubid identifies a specific column, refer to that in error
5389+
* messages. Otherwise, search to see if there's a user column of the
5390+
* type. (We assume system columns are never of interesting types.)
5391+
* The search is needed because an index containing an expression
5392+
* column of the target type will just be recorded as a whole-relation
5393+
* dependency. If we do not find a column of the type, the dependency
5394+
* must indicate that the type is transiently referenced in an index
5395+
* expression but not stored on disk, which we assume is OK, just as
5396+
* we do for references in views. (It could also be that the target
5397+
* type is embedded in some container type that is stored in an index
5398+
* column, but the previous recursion should catch such cases.)
5399+
*/
5400+
if (pg_depend->objsubid > 0 && pg_depend->objsubid <= tupleDesc->natts)
5401+
att = TupleDescAttr(tupleDesc, pg_depend->objsubid - 1);
5402+
else
5403+
{
5404+
int attno;
5405+
5406+
att = NULL;
5407+
for (attno = 1; attno <= tupleDesc->natts; attno++)
5408+
{
5409+
att = TupleDescAttr(tupleDesc, attno - 1);
5410+
if (att->atttypid == typeOid && !att->attisdropped)
5411+
break;
5412+
att = NULL;
5413+
}
5414+
if (att == NULL)
5415+
{
5416+
/* No such column, so assume OK */
5417+
relation_close(rel, AccessShareLock);
5418+
continue;
5419+
}
5420+
}
53875421

5422+
/*
5423+
* We definitely should reject if the relation has storage. If it's
5424+
* partitioned, then perhaps we don't have to reject: if there are
5425+
* partitions then we'll fail when we find one, else there is no
5426+
* stored data to worry about. However, it's possible that the type
5427+
* change would affect conclusions about whether the type is sortable
5428+
* or hashable and thus (if it's a partitioning column) break the
5429+
* partitioning rule. For now, reject for partitioned rels too.
5430+
*/
53885431
if (rel->rd_rel->relkind == RELKIND_RELATION ||
53895432
rel->rd_rel->relkind == RELKIND_MATVIEW ||
5390-
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
5433+
rel->rd_rel->relkind == RELKIND_INDEX ||
5434+
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
5435+
rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
53915436
{
53925437
if (origTypeName)
53935438
ereport(ERROR,

‎src/test/regress/expected/alter_table.out

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3037,6 +3037,13 @@ CREATE TYPE test_type1 AS (a int, b text);
30373037
CREATE TABLE test_tbl1 (x int, y test_type1);
30383038
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
30393039
ERROR: cannot alter type "test_type1" because column "test_tbl1.y" uses it
3040+
DROP TABLE test_tbl1;
3041+
CREATE TABLE test_tbl1 (x int, y text);
3042+
CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1));
3043+
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
3044+
ERROR: cannot alter type "test_type1" because column "test_tbl1_idx.row" uses it
3045+
DROP TABLE test_tbl1;
3046+
DROP TYPE test_type1;
30403047
CREATE TYPE test_type2 AS (a int, b text);
30413048
CREATE TABLE test_tbl2 OF test_type2;
30423049
CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2);
@@ -3148,7 +3155,8 @@ Typed table of type: test_type2
31483155
c | text | | |
31493156
Inherits: test_tbl2
31503157

3151-
DROP TABLE test_tbl2_subclass;
3158+
DROP TABLE test_tbl2_subclass, test_tbl2;
3159+
DROP TYPE test_type2;
31523160
CREATE TYPE test_typex AS (a int, b text);
31533161
CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
31543162
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
@@ -1917,6 +1917,14 @@ CREATE TYPE test_type1 AS (a int, b text);
19171917
CREATE TABLE test_tbl1 (x int, y test_type1);
19181918
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
19191919

1920+
DROP TABLE test_tbl1;
1921+
CREATE TABLE test_tbl1 (x int, y text);
1922+
CREATE INDEX test_tbl1_idx ON test_tbl1((row(x,y)::test_type1));
1923+
ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
1924+
1925+
DROP TABLE test_tbl1;
1926+
DROP TYPE test_type1;
1927+
19201928
CREATE TYPE test_type2 AS (a int, b text);
19211929
CREATE TABLE test_tbl2 OF test_type2;
19221930
CREATE TABLE test_tbl2_subclass () INHERITS (test_tbl2);
@@ -1944,7 +1952,8 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
19441952
\d test_tbl2
19451953
\d test_tbl2_subclass
19461954

1947-
DROP TABLE test_tbl2_subclass;
1955+
DROP TABLE test_tbl2_subclass, test_tbl2;
1956+
DROP TYPE test_type2;
19481957

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

0 commit comments

Comments
 (0)
0