Prevent a rowtype from being included in itself.

Eventually we might be able to allow that, but it's not clear how many
places need to be fixed to prevent infinite recursion when there's a direct
or indirect inclusion of a rowtype in itself.  One such place is
CheckAttributeType(), which will recurse to stack overflow in cases such as
those exhibited in bug #5950 from Alex Perepelica.  If we were sure it was
the only such place, we could easily modify the code added by this patch to
stop the recursion without a complaint ... but it probably isn't the only
such place.  Hence, throw error until such time as someone is excited
enough about this type of usage to put work into making it safe.

Back-patch as far as 8.3.  8.2 doesn't have the recursive call in
CheckAttributeType in the first place, so I see no need to add code there
in the absence of clear evidence of a problem elsewhere.

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 5dc90660deec2ee0fc6237e10efa2272f0b7b367..071266536df2683b03334663dcb510e82c5cf191 100644 (file)
--- 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 73b5e6b91c4ab03c2f40eadee03648c133b3622d..487dbbf24d247251d3078839009687f7c96af7bc 100644 (file)
--- 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 da491e14ef2f88d10f5141afd949f2017db66180..5d30b0fb20a6570414e221e5cf6a31090a44630d 100644 (file)
--- 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 2d6eb3c34ad392e140c26b2fd94489f70fac1d05..dac43b8adce08e35cfa09c02c5a00b023ddd95db 100644 (file)
--- 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 b14d61fd34fb6d25cf13064d1722a4c4e22778da..e8887db00e9718a8f97441ed786132e681915166 100644 (file)
--- 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 e041cec8439d539d0eb1adfe1fa9b4113b707c3b..26b5f03c179a5cd9d124fe02bbd2273086d1b461 100644 (file)
--- 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
 --