diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 75bdf7ba0c4..a7497e5c2a5 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3390,7 +3390,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, case AT_ReAddConstraint: /* Re-add pre-existing check * constraint */ ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, - false, true, lockmode); + true, true, lockmode); break; case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */ ATExecAddIndexConstraint(tab, rel, (IndexStmt *) cmd->def, lockmode); @@ -5764,13 +5764,6 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * AddRelationNewConstraints would normally assign different names to the * child constraints. To fix that, we must capture the name assigned at * the parent table and pass that down. - * - * When re-adding a previously existing constraint (during ALTER COLUMN TYPE), - * we don't need to recurse here, because recursion will be carried out at a - * higher level; the constraint name issue doesn't apply because the names - * have already been assigned and are just being re-used. We need a separate - * "is_readd" flag for that; just setting recurse=false would result in an - * error if there are child tables. */ static void ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, @@ -5798,7 +5791,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, */ newcons = AddRelationNewConstraints(rel, NIL, list_make1(copyObject(constr)), - recursing, /* allow_merge */ + recursing | is_readd, /* allow_merge */ !recursing, /* is_local */ is_readd); /* is_internal */ @@ -5842,10 +5835,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, /* * If adding a NO INHERIT constraint, no need to find our children. - * Likewise, in a re-add operation, we don't need to recurse (that will be - * handled at higher levels). */ - if (constr->is_no_inherit || is_readd) + if (constr->is_no_inherit) return; /* @@ -8204,10 +8195,30 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) def_item, tab->changedConstraintDefs) { Oid oldId = lfirst_oid(oid_item); + HeapTuple tup; + Form_pg_constraint con; Oid relid; Oid confrelid; + bool conislocal; + + tup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(oldId)); + if (!HeapTupleIsValid(tup)) /* should not happen */ + elog(ERROR, "cache lookup failed for constraint %u", oldId); + con = (Form_pg_constraint) GETSTRUCT(tup); + relid = con->conrelid; + confrelid = con->confrelid; + conislocal = con->conislocal; + ReleaseSysCache(tup); + + /* + * If the constraint is inherited (only), we don't want to inject a + * new definition here; it'll get recreated when ATAddCheckConstraint + * recurses from adding the parent table's constraint. But we had to + * carry the info this far so that we can drop the constraint below. + */ + if (!conislocal) + continue; - get_constraint_relation_oids(oldId, &relid, &confrelid); ATPostAlterTypeParse(oldId, relid, confrelid, (char *) lfirst(def_item), wqueue, lockmode, tab->rewrite); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index f8bfb46eade..48c0bbc3994 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -420,6 +420,7 @@ static Node *processIndirection(Node *node, deparse_context *context, static void printSubscripts(ArrayRef *aref, deparse_context *context); static char *get_relation_name(Oid relid); static char *generate_relation_name(Oid relid, List *namespaces); +static char *generate_qualified_relation_name(Oid relid); static char *generate_function_name(Oid funcid, int nargs, List *argnames, Oid *argtypes, bool has_variadic, bool *use_variadic_p); @@ -1298,7 +1299,9 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS) prettyFlags))); } -/* Internal version that returns a palloc'd C string; no pretty-printing */ +/* + * Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command + */ char * pg_get_constraintdef_string(Oid constraintId) { @@ -1347,10 +1350,16 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, initStringInfo(&buf); - if (fullCommand && OidIsValid(conForm->conrelid)) + if (fullCommand) { - appendStringInfo(&buf, "ALTER TABLE ONLY %s ADD CONSTRAINT %s ", - generate_relation_name(conForm->conrelid, NIL), + /* + * Currently, callers want ALTER TABLE (without ONLY) for CHECK + * constraints, and other types of constraints don't inherit anyway so + * it doesn't matter whether we say ONLY or not. Someday we might + * need to let callers specify whether to put ONLY in the command. + */ + appendStringInfo(&buf, "ALTER TABLE %s ADD CONSTRAINT %s ", + generate_qualified_relation_name(conForm->conrelid), quote_identifier(NameStr(conForm->conname))); } @@ -1874,28 +1883,9 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS) if (OidIsValid(sequenceId)) { - HeapTuple classtup; - Form_pg_class classtuple; - char *nspname; char *result; - /* Get the sequence's pg_class entry */ - classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(sequenceId)); - if (!HeapTupleIsValid(classtup)) - elog(ERROR, "cache lookup failed for relation %u", sequenceId); - classtuple = (Form_pg_class) GETSTRUCT(classtup); - - /* Get the namespace */ - nspname = get_namespace_name(classtuple->relnamespace); - if (!nspname) - elog(ERROR, "cache lookup failed for namespace %u", - classtuple->relnamespace); - - /* And construct the result string */ - result = quote_qualified_identifier(nspname, - NameStr(classtuple->relname)); - - ReleaseSysCache(classtup); + result = generate_qualified_relation_name(sequenceId); PG_RETURN_TEXT_P(string_to_text(result)); } @@ -9138,6 +9128,39 @@ generate_relation_name(Oid relid, List *namespaces) return result; } +/* + * generate_qualified_relation_name + * Compute the name to display for a relation specified by OID + * + * As above, but unconditionally schema-qualify the name. + */ +static char * +generate_qualified_relation_name(Oid relid) +{ + HeapTuple tp; + Form_pg_class reltup; + char *relname; + char *nspname; + char *result; + + tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tp)) + elog(ERROR, "cache lookup failed for relation %u", relid); + reltup = (Form_pg_class) GETSTRUCT(tp); + relname = NameStr(reltup->relname); + + nspname = get_namespace_name(reltup->relnamespace); + if (!nspname) + elog(ERROR, "cache lookup failed for namespace %u", + reltup->relnamespace); + + result = quote_qualified_identifier(nspname, relname); + + ReleaseSysCache(tp); + + return result; +} + /* * generate_function_name * Compute the name to display for a function specified by OID, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index eee26f659c8..635b018cd46 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1805,16 +1805,125 @@ where oid = 'test_storage'::regclass; t (1 row) --- ALTER TYPE with a check constraint and a child table (bug before Nov 2012) -CREATE TABLE test_inh_check (a float check (a > 10.2)); +-- ALTER COLUMN TYPE with a check constraint and a child table (bug #13779) +CREATE TABLE test_inh_check (a float check (a > 10.2), b float); CREATE TABLE test_inh_check_child() INHERITS(test_inh_check); +\d test_inh_check + Table "public.test_inh_check" + Column | Type | Modifiers +--------+------------------+----------- + a | double precision | + b | double precision | +Check constraints: + "test_inh_check_a_check" CHECK (a > 10.2::double precision) +Number of child tables: 1 (Use \d+ to list them.) + +\d test_inh_check_child + Table "public.test_inh_check_child" + Column | Type | Modifiers +--------+------------------+----------- + a | double precision | + b | double precision | +Check constraints: + "test_inh_check_a_check" CHECK (a > 10.2::double precision) +Inherits: test_inh_check + +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; + relname | conname | coninhcount | conislocal | connoinherit +----------------------+------------------------+-------------+------------+-------------- + test_inh_check | test_inh_check_a_check | 0 | t | f + test_inh_check_child | test_inh_check_a_check | 1 | f | f +(2 rows) + ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric; +\d test_inh_check + Table "public.test_inh_check" + Column | Type | Modifiers +--------+------------------+----------- + a | numeric | + b | double precision | +Check constraints: + "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) +Number of child tables: 1 (Use \d+ to list them.) + +\d test_inh_check_child + Table "public.test_inh_check_child" + Column | Type | Modifiers +--------+------------------+----------- + a | numeric | + b | double precision | +Check constraints: + "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) +Inherits: test_inh_check + +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; + relname | conname | coninhcount | conislocal | connoinherit +----------------------+------------------------+-------------+------------+-------------- + test_inh_check | test_inh_check_a_check | 0 | t | f + test_inh_check_child | test_inh_check_a_check | 1 | f | f +(2 rows) + +-- also try noinherit, local, and local+inherited cases +ALTER TABLE test_inh_check ADD CONSTRAINT bnoinherit CHECK (b > 100) NO INHERIT; +ALTER TABLE test_inh_check_child ADD CONSTRAINT blocal CHECK (b < 1000); +ALTER TABLE test_inh_check_child ADD CONSTRAINT bmerged CHECK (b > 1); +ALTER TABLE test_inh_check ADD CONSTRAINT bmerged CHECK (b > 1); +NOTICE: merging constraint "bmerged" with inherited definition +\d test_inh_check + Table "public.test_inh_check" + Column | Type | Modifiers +--------+------------------+----------- + a | numeric | + b | double precision | +Check constraints: + "bmerged" CHECK (b > 1::double precision) + "bnoinherit" CHECK (b > 100::double precision) NO INHERIT + "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) +Number of child tables: 1 (Use \d+ to list them.) + +\d test_inh_check_child + Table "public.test_inh_check_child" + Column | Type | Modifiers +--------+------------------+----------- + a | numeric | + b | double precision | +Check constraints: + "blocal" CHECK (b < 1000::double precision) + "bmerged" CHECK (b > 1::double precision) + "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) +Inherits: test_inh_check + +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; + relname | conname | coninhcount | conislocal | connoinherit +----------------------+------------------------+-------------+------------+-------------- + test_inh_check | bmerged | 0 | t | f + test_inh_check | bnoinherit | 0 | t | t + test_inh_check | test_inh_check_a_check | 0 | t | f + test_inh_check_child | blocal | 0 | t | f + test_inh_check_child | bmerged | 1 | t | f + test_inh_check_child | test_inh_check_a_check | 1 | f | f +(6 rows) + +ALTER TABLE test_inh_check ALTER COLUMN b TYPE numeric; +NOTICE: merging constraint "bmerged" with inherited definition \d test_inh_check Table "public.test_inh_check" Column | Type | Modifiers --------+---------+----------- a | numeric | + b | numeric | Check constraints: + "bmerged" CHECK (b::double precision > 1::double precision) + "bnoinherit" CHECK (b::double precision > 100::double precision) NO INHERIT "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) Number of child tables: 1 (Use \d+ to list them.) @@ -1823,10 +1932,27 @@ Table "public.test_inh_check_child" Column | Type | Modifiers --------+---------+----------- a | numeric | + b | numeric | Check constraints: + "blocal" CHECK (b::double precision < 1000::double precision) + "bmerged" CHECK (b::double precision > 1::double precision) "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) Inherits: test_inh_check +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; + relname | conname | coninhcount | conislocal | connoinherit +----------------------+------------------------+-------------+------------+-------------- + test_inh_check | bmerged | 0 | t | f + test_inh_check | bnoinherit | 0 | t | t + test_inh_check | test_inh_check_a_check | 0 | t | f + test_inh_check_child | blocal | 0 | t | f + test_inh_check_child | bmerged | 1 | t | f + test_inh_check_child | test_inh_check_a_check | 1 | f | f +(6 rows) + -- check for rollback of ANALYZE corrupting table property flags (bug #11638) CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text); CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index aadd3953190..429f792c6c7 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1252,12 +1252,40 @@ select reltoastrelid <> 0 as has_toast_table from pg_class where oid = 'test_storage'::regclass; --- ALTER TYPE with a check constraint and a child table (bug before Nov 2012) -CREATE TABLE test_inh_check (a float check (a > 10.2)); +-- ALTER COLUMN TYPE with a check constraint and a child table (bug #13779) +CREATE TABLE test_inh_check (a float check (a > 10.2), b float); CREATE TABLE test_inh_check_child() INHERITS(test_inh_check); +\d test_inh_check +\d test_inh_check_child +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric; \d test_inh_check \d test_inh_check_child +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; +-- also try noinherit, local, and local+inherited cases +ALTER TABLE test_inh_check ADD CONSTRAINT bnoinherit CHECK (b > 100) NO INHERIT; +ALTER TABLE test_inh_check_child ADD CONSTRAINT blocal CHECK (b < 1000); +ALTER TABLE test_inh_check_child ADD CONSTRAINT bmerged CHECK (b > 1); +ALTER TABLE test_inh_check ADD CONSTRAINT bmerged CHECK (b > 1); +\d test_inh_check +\d test_inh_check_child +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; +ALTER TABLE test_inh_check ALTER COLUMN b TYPE numeric; +\d test_inh_check +\d test_inh_check_child +select relname, conname, coninhcount, conislocal, connoinherit + from pg_constraint c, pg_class r + where relname like 'test_inh_check%' and c.conrelid = r.oid + order by 1, 2; -- check for rollback of ANALYZE corrupting table property flags (bug #11638) CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);