diff --git a/doc/src/sgml/ref/alter_domain.sgml b/doc/src/sgml/ref/alter_domain.sgml index 1111da12f8..da024558f4 100644 --- a/doc/src/sgml/ref/alter_domain.sgml +++ b/doc/src/sgml/ref/alter_domain.sgml @@ -1,5 +1,5 @@ @@ -197,6 +197,19 @@ ALTER DOMAIN name + + Notes + + + Currently, ALTER DOMAIN ADD CONSTRAINT and + ALTER DOMAIN SET NOT NULL will fail if the named domain or + any derived domain is used within a composite-type column of any + table in the database. They should eventually be improved to be + able to verify the new constraint for such nested columns. + + + + Examples diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1bbf17789..eecd4943ae 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.220 2007/05/11 17:57:12 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.221 2007/05/11 20:16:36 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -206,8 +206,6 @@ static void ATSimpleRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse); static void ATOneLevelRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd); -static void find_composite_type_dependencies(Oid typeOid, - const char *origTblName); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd); static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel, @@ -2410,7 +2408,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap) */ if (newrel) find_composite_type_dependencies(oldrel->rd_rel->reltype, - RelationGetRelationName(oldrel)); + RelationGetRelationName(oldrel), + NULL); /* * Generate the constraint and default execution states @@ -2812,16 +2811,21 @@ ATOneLevelRecursion(List **wqueue, Relation rel, /* * find_composite_type_dependencies * - * Check to see if a table's rowtype is being used as a column in some + * Check to see if a composite type is being used as a column in some * other table (possibly nested several levels deep in composite types!). * Eventually, we'd like to propagate the check or rewrite operation * into other such tables, but for now, just error out if we find any. * + * Caller should provide either a table name or a type name (not both) to + * report in the error message, if any. + * * We assume that functions and views depending on the type are not reasons * to reject the ALTER. (How safe is this really?) */ -static void -find_composite_type_dependencies(Oid typeOid, const char *origTblName) +void +find_composite_type_dependencies(Oid typeOid, + const char *origTblName, + const char *origTypeName) { Relation depRel; ScanKeyData key[2]; @@ -2864,12 +2868,20 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName) if (rel->rd_rel->relkind == RELKIND_RELATION) { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype", - origTblName, - RelationGetRelationName(rel), - NameStr(att->attname)))); + if (origTblName) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter table \"%s\" because column \"%s\".\"%s\" uses its rowtype", + origTblName, + RelationGetRelationName(rel), + NameStr(att->attname)))); + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type \"%s\" because column \"%s\".\"%s\" uses it", + origTypeName, + RelationGetRelationName(rel), + NameStr(att->attname)))); } else if (OidIsValid(rel->rd_rel->reltype)) { @@ -2878,7 +2890,7 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName) * recursively check for indirect dependencies via its rowtype. */ find_composite_type_dependencies(rel->rd_rel->reltype, - origTblName); + origTblName, origTypeName); } relation_close(rel, AccessShareLock); @@ -2894,7 +2906,7 @@ find_composite_type_dependencies(Oid typeOid, const char *origTblName) */ arrayOid = get_array_type(typeOid); if (OidIsValid(arrayOid)) - find_composite_type_dependencies(arrayOid, origTblName); + find_composite_type_dependencies(arrayOid, origTblName, origTypeName); } diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 7911f6df3a..915c669c97 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.102 2007/05/11 17:57:12 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.103 2007/05/11 20:16:54 tgl Exp $ * * DESCRIPTION * The "DefineFoo" routines take the parse tree and pick out the @@ -1805,6 +1805,10 @@ AlterDomainAddConstraint(List *names, Node *newConstraint) * the domain type. We have opened each rel and acquired the specified lock * type on it. * + * We support nested domains by including attributes that are of derived + * domain types. Current callers do not need to distinguish between attributes + * that are of exactly the given domain and those that are of derived domains. + * * XXX this is completely broken because there is no way to lock the domain * to prevent columns from being added or dropped while our command runs. * We can partially protect against column drops by locking relations as we @@ -1814,9 +1818,11 @@ AlterDomainAddConstraint(List *names, Node *newConstraint) * trivial risk of deadlock. We can minimize but not eliminate the deadlock * risk by using the weakest suitable lock (ShareLock for most callers). * - * XXX to support domains over domains, we'd need to make this smarter, - * or make its callers smarter, so that we could find columns of derived - * domains. Arrays of domains would be a problem too. + * XXX the API for this is not sufficient to support checking domain values + * that are inside composite types or arrays. Currently we just error out + * if a composite type containing the target domain is stored anywhere. + * There are not currently arrays of domains; if there were, we could take + * the same approach, but it'd be nicer to fix it properly. * * Generally used for retrieving a list of tests when adding * new constraints to a domain. @@ -1858,7 +1864,23 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) Form_pg_attribute pg_att; int ptr; - /* Ignore dependees that aren't user columns of relations */ + /* Check for directly dependent types --- must be domains */ + if (pg_depend->classid == TypeRelationId) + { + Assert(get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN); + /* + * Recursively add dependent columns to the output list. This + * is a bit inefficient since we may fail to combine RelToCheck + * entries when attributes of the same rel have different derived + * domain types, but it's probably not worth improving. + */ + result = list_concat(result, + get_rels_with_domain(pg_depend->objid, + lockmode)); + continue; + } + + /* Else, ignore dependees that aren't user columns of relations */ /* (we assume system columns are never of domain types) */ if (pg_depend->classid != RelationRelationId || pg_depend->objsubid <= 0) @@ -1884,7 +1906,16 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) /* Acquire requested lock on relation */ rel = relation_open(pg_depend->objid, lockmode); - /* It could be a view or composite type; if so ignore it */ + /* + * Check to see if rowtype is stored anyplace as a composite-type + * column; if so we have to fail, for now anyway. + */ + if (OidIsValid(rel->rd_rel->reltype)) + find_composite_type_dependencies(rel->rd_rel->reltype, + NULL, + format_type_be(domainOid)); + + /* Otherwise we can ignore views, composite types, etc */ if (rel->rd_rel->relkind != RELKIND_RELATION) { relation_close(rel, lockmode); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index fc428aad91..3acbe84b52 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.32 2007/01/05 22:19:54 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.33 2007/05/11 20:17:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -45,6 +45,10 @@ extern void renameatt(Oid myrelid, extern void renamerel(Oid myrelid, const char *newrelname); +extern void find_composite_type_dependencies(Oid typeOid, + const char *origTblName, + const char *origTypeName); + extern AttrNumber *varattnos_map(TupleDesc old, TupleDesc new); extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema); extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index e01fc7cadc..c7d5b50334 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -438,3 +438,32 @@ select doubledecrement(3); -- good 1 (1 row) +-- Check that ALTER DOMAIN tests columns of derived types +create domain posint as int4; +-- Currently, this doesn't work for composite types, but verify it complains +create type ddtest1 as (f1 posint); +create table ddtest2(f1 ddtest1); +insert into ddtest2 values(row(-1)); +alter domain posint add constraint c1 check(value >= 0); +ERROR: cannot alter type "posint" because column "ddtest2"."f1" uses it +drop table ddtest2; +create table ddtest2(f1 ddtest1[]); +insert into ddtest2 values('{(-1)}'); +alter domain posint add constraint c1 check(value >= 0); +ERROR: cannot alter type "posint" because column "ddtest2"."f1" uses it +drop table ddtest2; +alter domain posint add constraint c1 check(value >= 0); +create domain posint2 as posint check (value % 2 = 0); +create table ddtest2(f1 posint2); +insert into ddtest2 values(11); -- fail +ERROR: value for domain posint2 violates check constraint "posint2_check" +insert into ddtest2 values(-2); -- fail +ERROR: value for domain posint2 violates check constraint "c1" +insert into ddtest2 values(2); +alter domain posint add constraint c2 check(value >= 10); -- fail +ERROR: column "f1" of table "ddtest2" contains values that violate the new constraint +alter domain posint add constraint c2 check(value > 0); -- OK +drop table ddtest2; +drop type ddtest1; +drop domain posint cascade; +NOTICE: drop cascades to type posint2 diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 4e103172ed..7da9971991 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -351,3 +351,34 @@ select doubledecrement(0); -- fail before call select doubledecrement(1); -- fail at assignment to v select doubledecrement(2); -- fail at return select doubledecrement(3); -- good + +-- Check that ALTER DOMAIN tests columns of derived types + +create domain posint as int4; + +-- Currently, this doesn't work for composite types, but verify it complains +create type ddtest1 as (f1 posint); +create table ddtest2(f1 ddtest1); +insert into ddtest2 values(row(-1)); +alter domain posint add constraint c1 check(value >= 0); +drop table ddtest2; + +create table ddtest2(f1 ddtest1[]); +insert into ddtest2 values('{(-1)}'); +alter domain posint add constraint c1 check(value >= 0); +drop table ddtest2; + +alter domain posint add constraint c1 check(value >= 0); + +create domain posint2 as posint check (value % 2 = 0); +create table ddtest2(f1 posint2); +insert into ddtest2 values(11); -- fail +insert into ddtest2 values(-2); -- fail +insert into ddtest2 values(2); + +alter domain posint add constraint c2 check(value >= 10); -- fail +alter domain posint add constraint c2 check(value > 0); -- OK + +drop table ddtest2; +drop type ddtest1; +drop domain posint cascade;