diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2900f47fb72883c716f9331dc0909481fa0f778c..2a13f9c53e92dcd47ae71a5b7fb025329fdb6608 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3261,7 +3261,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, break; 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); @@ -5578,13 +5578,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, @@ -5612,7 +5605,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 */ /* Add each to-be-validated constraint to Phase 3's queue */ @@ -5655,10 +5648,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; /* @@ -7874,11 +7865,31 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) forboth(oid_item, tab->changedConstraintOids, def_item, tab->changedConstraintDefs) { - Oid oldId = lfirst_oid(oid_item); - Oid relid; - Oid confrelid; + 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 5bac2f03a6ceceaab3df51a70407abe80c40f268..2d28b53a8b0515369a6364210a9a6dfe0de6cb18 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -257,6 +257,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 *is_variadic); static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2); @@ -1115,7 +1116,9 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS) false, prettyFlags))); } -/* Internal version that returns a palloc'd C string */ +/* + * Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command + */ char * pg_get_constraintdef_string(Oid constraintId) { @@ -1137,10 +1140,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))); } @@ -1660,28 +1669,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)); } @@ -7380,6 +7370,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 459e4c0f62c2dcd6d6a0e6a03ce1ae29e13c53ff..42528451306665d0280903931c3d95d7b3698ef6 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1835,16 +1835,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.) @@ -1853,10 +1962,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); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "check_fk_presence_1_pkey" for table "check_fk_presence_1" diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 4f8e5b22047251ff28ffa3050f59d9c2d14bd006..6b054d48cecabea43b39d733258f67b120526906 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);