diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f79044f39f..f64dfffef0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -328,7 +328,8 @@ static void AlterSeqNamespaces(Relation classRel, Relation rel, LOCKMODE lockmode); static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode); -static ObjectAddress ATExecValidateConstraint(Relation rel, char *constrName, +static ObjectAddress ATExecValidateConstraint(AlteredTableInfo *tab, + Relation rel, char *constrName, bool recurse, bool recursing, LOCKMODE lockmode); static int transformColumnNameList(Oid relId, List *colList, int16 *attnums, Oid *atttypids); @@ -4500,13 +4501,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, address = ATExecAlterConstraint(rel, cmd, false, false, lockmode); break; case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ - address = ATExecValidateConstraint(rel, cmd->name, false, false, - lockmode); + address = ATExecValidateConstraint(tab, rel, cmd->name, false, + false, lockmode); break; case AT_ValidateConstraintRecurse: /* VALIDATE CONSTRAINT with * recursion */ - address = ATExecValidateConstraint(rel, cmd->name, true, false, - lockmode); + address = ATExecValidateConstraint(tab, rel, cmd->name, true, + false, lockmode); break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATExecDropConstraint(rel, cmd->name, cmd->behavior, @@ -9727,8 +9728,9 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, * was already validated, InvalidObjectAddress is returned. */ static ObjectAddress -ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, - bool recursing, LOCKMODE lockmode) +ATExecValidateConstraint(AlteredTableInfo *tab, Relation rel, + char *constrName, bool recurse, bool recursing, + LOCKMODE lockmode) { Relation conrel; SysScanDesc scan; @@ -9779,22 +9781,50 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, if (con->contype == CONSTRAINT_FOREIGN) { - Relation refrel; - /* - * Triggers are already in place on both tables, so a concurrent - * write that alters the result here is not possible. Normally we - * can run a query here to do the validation, which would only - * require AccessShareLock. In some cases, it is possible that we - * might need to fire triggers to perform the check, so we take a - * lock at RowShareLock level just in case. + * If a table rewrite is pending then we may not be able to + * perform a foreign key constriant validation right now. We'll + * just mark that the validation must be done after the rewrite + * instead. */ - refrel = table_open(con->confrelid, RowShareLock); + if (tab->rewrite > 0) + { + NewConstraint *newcon; + Constraint *fkconstraint; + + fkconstraint = makeNode(Constraint); + /* for now this is all we need */ + fkconstraint->conname = constrName; + + newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); + newcon->name = constrName; + newcon->contype = CONSTR_FOREIGN; + newcon->refrelid = con->confrelid; + newcon->refindid = con->conindid; + newcon->conid = con->oid; + newcon->qual = (Node *) fkconstraint; + + tab->constraints = lappend(tab->constraints, newcon); + } + else + { + Relation refrel; - validateForeignKeyConstraint(constrName, rel, refrel, - con->conindid, - con->oid); - table_close(refrel, NoLock); + /* + * Triggers are already in place on both tables, so a concurrent + * write that alters the result here is not possible. Normally we + * can run a query here to do the validation, which would only + * require AccessShareLock. In some cases, it is possible that we + * might need to fire triggers to perform the check, so we take a + * lock at RowShareLock level just in case. + */ + refrel = table_open(con->confrelid, RowShareLock); + + validateForeignKeyConstraint(constrName, rel, refrel, + con->conindid, + con->oid); + table_close(refrel, NoLock); + } /* * We disallow creating invalid foreign keys to or from @@ -9844,7 +9874,7 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, /* find_all_inheritors already got lock */ childrel = table_open(childoid, NoLock); - ATExecValidateConstraint(childrel, constrName, false, + ATExecValidateConstraint(tab, childrel, constrName, false, true, lockmode); table_close(childrel, NoLock); } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 002079601f..a3b8e7c044 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -997,6 +997,12 @@ alter table atacc1 add column b float8 not null default random(), add primary key(a); drop table atacc1; +-- additionally, we've seen issues with foreign key validation not being +-- properly delayed until after a table rewrite. Check that works ok. +create table atacc1 (a int primary key); +alter table atacc1 add constraint atacc1_fkey foreign key (a) references atacc1 (a) not valid; +alter table atacc1 validate constraint atacc1_fkey, alter A type bigint; +drop table atacc1; -- something a little more complicated create table atacc1 ( test int, test2 int); -- add a primary key constraint diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index ec272d78f8..6f9091060b 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -757,6 +757,13 @@ alter table atacc1 add primary key(a); drop table atacc1; +-- additionally, we've seen issues with foreign key validation not being +-- properly delayed until after a table rewrite. Check that works ok. +create table atacc1 (a int primary key); +alter table atacc1 add constraint atacc1_fkey foreign key (a) references atacc1 (a) not valid; +alter table atacc1 validate constraint atacc1_fkey, alter A type bigint; +drop table atacc1; + -- something a little more complicated create table atacc1 ( test int, test2 int); -- add a primary key constraint