diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f1daf4b148..c6447e2570 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -288,8 +288,9 @@ 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, - bool recurse, bool recursing, LOCKMODE lockmode); +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); static int transformFkeyGetPrimaryKey(Relation pkrel, Oid *indexOid, @@ -3582,13 +3583,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, @@ -6851,8 +6852,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; @@ -6904,22 +6906,45 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, if (con->contype == CONSTRAINT_FOREIGN) { - Relation refrel; + 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 = HeapTupleGetOid(tuple); + newcon->qual = (Node *) fkconstraint; + + tab->constraints = lappend(tab->constraints, newcon); + } + else + { - /* - * 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 = heap_open(con->confrelid, RowShareLock); + Relation refrel; - validateForeignKeyConstraint(constrName, rel, refrel, - con->conindid, - HeapTupleGetOid(tuple)); - heap_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 = heap_open(con->confrelid, RowShareLock); + + validateForeignKeyConstraint(constrName, rel, refrel, + con->conindid, + HeapTupleGetOid(tuple)); + heap_close(refrel, NoLock); + } /* * Foreign keys do not inherit, so we purposely ignore the @@ -6968,7 +6993,7 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, /* find_all_inheritors already got lock */ childrel = heap_open(childoid, NoLock); - ATExecValidateConstraint(childrel, constrName, false, + ATExecValidateConstraint(tab, childrel, constrName, false, true, lockmode); heap_close(childrel, NoLock); } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 46402b4a6d..1b54e01f95 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -932,6 +932,12 @@ ERROR: column "test2" contains null values -- now add a primary key column with a default (succeeds). alter table atacc1 add column test2 int default 0 primary key; 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 421d4c7d31..9a1dc69b64 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -710,6 +710,13 @@ alter table atacc1 add column test2 int primary key; alter table atacc1 add column test2 int default 0 primary key; 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