diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 5dc90660dee..071266536df 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -403,7 +403,8 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind) for (i = 0; i < natts; i++) { CheckAttributeType(NameStr(tupdesc->attrs[i]->attname), - tupdesc->attrs[i]->atttypid); + tupdesc->attrs[i]->atttypid, + NIL /* assume we're creating a new rowtype */); } } @@ -411,14 +412,23 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind) * CheckAttributeType * * Verify that the proposed datatype of an attribute is legal. - * This is needed because there are types (and pseudo-types) + * This is needed mainly because there are types (and pseudo-types) * in the catalogs that we do not support as elements of real tuples. + * We also check some other properties required of a table column. + * + * If the attribute is being proposed for addition to an existing table or + * composite type, pass a one-element list of the rowtype OID as + * containing_rowtypes. When checking a to-be-created rowtype, it's + * sufficient to pass NIL, because there could not be any recursive reference + * to a not-yet-existing rowtype. * -------------------------------- */ void -CheckAttributeType(const char *attname, Oid atttypid) +CheckAttributeType(const char *attname, Oid atttypid, + List *containing_rowtypes) { char att_typtype = get_typtype(atttypid); + Oid att_typelem; if (atttypid == UNKNOWNOID) { @@ -454,6 +464,20 @@ CheckAttributeType(const char *attname, Oid atttypid) TupleDesc tupdesc; int i; + /* + * Check for self-containment. Eventually we might be able to allow + * this (just return without complaint, if so) but it's not clear how + * many other places would require anti-recursion defenses before it + * would be safe to allow tables to contain their own rowtype. + */ + if (list_member_oid(containing_rowtypes, atttypid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("composite type %s cannot be made a member of itself", + format_type_be(atttypid)))); + + containing_rowtypes = lcons_oid(atttypid, containing_rowtypes); + relation = relation_open(get_typ_typrelid(atttypid), AccessShareLock); tupdesc = RelationGetDescr(relation); @@ -464,10 +488,21 @@ CheckAttributeType(const char *attname, Oid atttypid) if (attr->attisdropped) continue; - CheckAttributeType(NameStr(attr->attname), attr->atttypid); + CheckAttributeType(NameStr(attr->attname), attr->atttypid, + containing_rowtypes); } relation_close(relation, AccessShareLock); + + containing_rowtypes = list_delete_first(containing_rowtypes); + } + else if (OidIsValid((att_typelem = get_element_type(atttypid)))) + { + /* + * Must recurse into array types, too, in case they are composite. + */ + CheckAttributeType(attname, att_typelem, + containing_rowtypes); } } diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 73b5e6b91c4..487dbbf24d2 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -247,7 +247,7 @@ ConstructTupleDescriptor(Relation heapRelation, * whether a table column is of a safe type (which is why we * needn't check for the non-expression case). */ - CheckAttributeType(NameStr(to->attname), to->atttypid); + CheckAttributeType(NameStr(to->attname), to->atttypid, NIL); } /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index da491e14ef2..5d30b0fb20a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3613,7 +3613,8 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel, typeOid = HeapTupleGetOid(typeTuple); /* make sure datatype is legal for a column */ - CheckAttributeType(colDef->colname, typeOid); + CheckAttributeType(colDef->colname, typeOid, + list_make1_oid(rel->rd_rel->reltype)); /* construct new attribute's pg_attribute entry */ attribute.attrelid = myrelid; @@ -5606,7 +5607,8 @@ ATPrepAlterColumnType(List **wqueue, targettype = typenameTypeId(NULL, typename, &targettypmod); /* make sure datatype is legal for a column */ - CheckAttributeType(colName, targettype); + CheckAttributeType(colName, targettype, + list_make1_oid(rel->rd_rel->reltype)); /* * Set up an expression to transform the old data value to the new type. diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 2d6eb3c34ad..dac43b8adce 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -105,6 +105,7 @@ extern Form_pg_attribute SystemAttributeByName(const char *attname, extern void CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind); -extern void CheckAttributeType(const char *attname, Oid atttypid); +extern void CheckAttributeType(const char *attname, Oid atttypid, + List *containing_rowtypes); #endif /* HEAP_H */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index b14d61fd34f..e8887db00e9 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1456,6 +1456,18 @@ select * from another; (3 rows) drop table another; +-- disallow recursive containment of row types +create temp table recur1 (f1 int); +alter table recur1 add column f2 recur1; -- fails +ERROR: composite type recur1 cannot be made a member of itself +alter table recur1 add column f2 recur1[]; -- fails +ERROR: composite type recur1 cannot be made a member of itself +create temp table recur2 (f1 int, f2 recur1); +alter table recur1 add column f2 recur2; -- fails +ERROR: composite type recur1 cannot be made a member of itself +alter table recur1 add column f2 int; +alter table recur1 alter column f2 type recur2; -- fails +ERROR: composite type recur1 cannot be made a member of itself -- -- alter function -- diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index e041cec8439..26b5f03c179 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1081,6 +1081,15 @@ select * from another; drop table another; +-- disallow recursive containment of row types +create temp table recur1 (f1 int); +alter table recur1 add column f2 recur1; -- fails +alter table recur1 add column f2 recur1[]; -- fails +create temp table recur2 (f1 int, f2 recur1); +alter table recur1 add column f2 recur2; -- fails +alter table recur1 add column f2 int; +alter table recur1 alter column f2 type recur2; -- fails + -- -- alter function --