static void ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
bool recurse, LOCKMODE lockmode,
AlterTableUtilityContext *context);
+static void verifyNotNullPKCompatible(HeapTuple tuple, const char *colname);
static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode);
static ObjectAddress ATExecAddStatistics(AlteredTableInfo *tab, Relation rel,
}
/*
- * Prepare to add a primary key on table, by adding not-null constraints
+ * Prepare to add a primary key on a table, by adding not-null constraints
* on all columns.
+ *
+ * The not-null constraints for a primary key must cover the whole inheritance
+ * hierarchy (failing to ensure that leads to funny corner cases). For the
+ * normal case where we're asked to recurse, this routine ensures that the
+ * not-null constraints either exist already, or queues a requirement for them
+ * to be created by phase 2.
+ *
+ * For the case where we're asked not to recurse, we verify that a not-null
+ * constraint exists on each column of each (direct) child table, throwing an
+ * error if not. Not throwing an error would also work, because a not-null
+ * constraint would be created anyway, but it'd cause a silent scan of the
+ * child table to verify absence of nulls. We prefer to let the user know so
+ * that they can add the constraint manually without having to hold
+ * AccessExclusiveLock while at it.
+ *
+ * However, it's also important that we do not acquire locks on children if
+ * the not-null constraints already exist on the parent, to avoid risking
+ * deadlocks during parallel pg_restore of PKs on partitioned tables.
*/
static void
ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
AlterTableUtilityContext *context)
{
Constraint *pkconstr;
+ List *children;
+ bool got_children = false;
pkconstr = castNode(Constraint, cmd->def);
if (pkconstr->contype != CONSTR_PRIMARY)
return;
- /*
- * If not recursing, we must ensure that all children have a NOT NULL
- * constraint on the columns, and error out if not.
- */
- if (!recurse)
- {
- List *children;
-
- children = find_inheritance_children(RelationGetRelid(rel),
- lockmode);
- foreach_oid(childrelid, children)
- {
- foreach_node(String, attname, pkconstr->keys)
- {
- HeapTuple tup;
- Form_pg_attribute attrForm;
-
- tup = SearchSysCacheAttName(childrelid, strVal(attname));
- if (!tup)
- elog(ERROR, "cache lookup failed for attribute %s of relation %u",
- strVal(attname), childrelid);
- attrForm = (Form_pg_attribute) GETSTRUCT(tup);
- if (!attrForm->attnotnull)
- ereport(ERROR,
- errmsg("column \"%s\" of table \"%s\" is not marked NOT NULL",
- strVal(attname), get_rel_name(childrelid)));
- ReleaseSysCache(tup);
- }
- }
- }
-
/* Verify that columns are not-null, or request that they be made so */
foreach_node(String, column, pkconstr->keys)
{
tuple = findNotNullConstraint(RelationGetRelid(rel), strVal(column));
if (tuple != NULL)
{
- Form_pg_constraint conForm = (Form_pg_constraint) GETSTRUCT(tuple);
-
- /* a NO INHERIT constraint is no good */
- if (conForm->connoinherit)
- ereport(ERROR,
- errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("cannot create primary key on column \"%s\"",
- strVal(column)),
- /*- translator: third %s is a constraint characteristic such as NOT VALID */
- errdetail("The constraint \"%s\" on column \"%s\", marked %s, is incompatible with a primary key.",
- NameStr(conForm->conname), strVal(column), "NO INHERIT"),
- errhint("You will need to make it inheritable using %s.",
- "ALTER TABLE ... ALTER CONSTRAINT ... INHERIT"));
-
- /* an unvalidated constraint is no good */
- if (!conForm->convalidated)
- ereport(ERROR,
- errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("cannot create primary key on column \"%s\"",
- strVal(column)),
- /*- translator: third %s is a constraint characteristic such as NOT VALID */
- errdetail("The constraint \"%s\" on column \"%s\", marked %s, is incompatible with a primary key.",
- NameStr(conForm->conname), strVal(column), "NOT VALID"),
- errhint("You will need to validate it using %s.",
- "ALTER TABLE ... VALIDATE CONSTRAINT"));
+ verifyNotNullPKCompatible(tuple, strVal(column));
/* All good with this one; don't request another */
heap_freetuple(tuple);
continue;
}
+ else if (!recurse)
+ {
+ /*
+ * No constraint on this column. Asked not to recurse, we won't
+ * create one here, but verify that all children have one.
+ */
+ if (!got_children)
+ {
+ children = find_inheritance_children(RelationGetRelid(rel),
+ lockmode);
+ /* only search for children on the first time through */
+ got_children = true;
+ }
+
+ foreach_oid(childrelid, children)
+ {
+ HeapTuple tup;
+
+ tup = findNotNullConstraint(childrelid, strVal(column));
+ if (!tup)
+ ereport(ERROR,
+ errmsg("column \"%s\" of table \"%s\" is not marked NOT NULL",
+ strVal(column), get_rel_name(childrelid)));
+ /* verify it's good enough */
+ verifyNotNullPKCompatible(tup, strVal(column));
+ }
+ }
/* This column is not already not-null, so add it to the queue */
nnconstr = makeNotNullConstraint(column);
newcmd = makeNode(AlterTableCmd);
newcmd->subtype = AT_AddConstraint;
+ /* note we force recurse=true here; see above */
newcmd->recurse = true;
newcmd->def = (Node *) nnconstr;
}
}
+/*
+ * Verify whether the given not-null constraint is compatible with a
+ * primary key. If not, an error is thrown.
+ */
+static void
+verifyNotNullPKCompatible(HeapTuple tuple, const char *colname)
+{
+ Form_pg_constraint conForm = (Form_pg_constraint) GETSTRUCT(tuple);
+
+ if (conForm->contype != CONSTRAINT_NOTNULL)
+ elog(ERROR, "constraint %u is not a not-null constraint", conForm->oid);
+
+ /* a NO INHERIT constraint is no good */
+ if (conForm->connoinherit)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot create primary key on column \"%s\"", colname),
+ /*- translator: fourth %s is a constraint characteristic such as NOT VALID */
+ errdetail("The constraint \"%s\" on column \"%s\" of table \"%s\", marked %s, is incompatible with a primary key.",
+ NameStr(conForm->conname), colname,
+ get_rel_name(conForm->conrelid), "NO INHERIT"),
+ errhint("You might need to make the existing constraint inheritable using %s.",
+ "ALTER TABLE ... ALTER CONSTRAINT ... INHERIT"));
+
+ /* an unvalidated constraint is no good */
+ if (!conForm->convalidated)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot create primary key on column \"%s\"", colname),
+ /*- translator: fourth %s is a constraint characteristic such as NOT VALID */
+ errdetail("The constraint \"%s\" on column \"%s\" of table \"%s\", marked %s, is incompatible with a primary key.",
+ NameStr(conForm->conname), colname,
+ get_rel_name(conForm->conrelid), "NOT VALID"),
+ errhint("You might need to validate it using %s.",
+ "ALTER TABLE ... VALIDATE CONSTRAINT"));
+}
+
/*
* ALTER TABLE ADD INDEX
*
-- can't override
ALTER TABLE ATACC1 ADD CONSTRAINT ditto NOT NULL a;
ERROR: cannot change NO INHERIT status of NOT NULL constraint "a_is_not_null" on relation "atacc2"
+HINT: You might need to make the existing constraint inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
-- dropping the NO INHERIT constraint allows this to work
ALTER TABLE ATACC2 DROP CONSTRAINT a_is_not_null;
ALTER TABLE ATACC1 ADD CONSTRAINT ditto NOT NULL a;
create table cnn_pk (a int not null no inherit);
alter table cnn_pk add primary key (a);
ERROR: cannot create primary key on column "a"
-DETAIL: The constraint "cnn_pk_a_not_null" on column "a", marked NO INHERIT, is incompatible with a primary key.
-HINT: You will need to make it inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
+DETAIL: The constraint "cnn_pk_a_not_null" on column "a" of table "cnn_pk", marked NO INHERIT, is incompatible with a primary key.
+HINT: You might need to make the existing constraint inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
drop table cnn_pk;
-- Ensure partitions are scanned for null values when adding a PK
create table cnn2_parted(a int) partition by list (a);
-- If we have an invalid constraint, we can't have another
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn1 NOT NULL a NOT VALID NO INHERIT;
ERROR: cannot change NO INHERIT status of NOT NULL constraint "nn" on relation "notnull_tbl1"
+HINT: You might need to make the existing constraint inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn NOT NULL a;
ERROR: incompatible NOT VALID constraint "nn" on relation "notnull_tbl1"
-HINT: You will need to use ALTER TABLE ... VALIDATE CONSTRAINT to validate it.
+HINT: You might need to validate it using ALTER TABLE ... VALIDATE CONSTRAINT.
-- cannot add primary key on a column with an invalid not-null
ALTER TABLE notnull_tbl1 ADD PRIMARY KEY (a);
ERROR: cannot create primary key on column "a"
-DETAIL: The constraint "nn" on column "a", marked NOT VALID, is incompatible with a primary key.
-HINT: You will need to validate it using ALTER TABLE ... VALIDATE CONSTRAINT.
+DETAIL: The constraint "nn" on column "a" of table "notnull_tbl1", marked NOT VALID, is incompatible with a primary key.
+HINT: You might need to validate it using ALTER TABLE ... VALIDATE CONSTRAINT.
-- ALTER column SET NOT NULL validates an invalid constraint (but this fails
-- because of rows with null values)
ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL;
ALTER TABLE notnull_inhparent ADD CONSTRAINT nn NOT NULL i NOT VALID;
ALTER TABLE notnull_inhchild ADD CONSTRAINT nn1 NOT NULL i; -- error
ERROR: incompatible NOT VALID constraint "nn" on relation "notnull_inhchild"
-HINT: You will need to use ALTER TABLE ... VALIDATE CONSTRAINT to validate it.
+HINT: You might need to validate it using ALTER TABLE ... VALIDATE CONSTRAINT.
EXECUTE get_nnconstraint_info('{notnull_inhparent, notnull_inhchild, notnull_inhgrand}');
tabname | conname | convalidated | conislocal | coninhcount
-------------------+---------+--------------+------------+-------------
ALTER TABLE pp_nn_1 VALIDATE CONSTRAINT nn1;
ALTER TABLE pp_nn ATTACH PARTITION pp_nn_1 FOR VALUES IN (NULL,5); --ok
DROP TABLE pp_nn;
+-- Try a partition with an invalid constraint and create a PK on the parent.
+CREATE TABLE pp_nn (a int) PARTITION BY HASH (a);
+CREATE TABLE pp_nn_1 PARTITION OF pp_nn FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+ALTER TABLE pp_nn_1 ADD CONSTRAINT nn NOT NULL a NOT VALID;
+ALTER TABLE ONLY pp_nn ADD PRIMARY KEY (a);
+ERROR: cannot create primary key on column "a"
+DETAIL: The constraint "nn" on column "a" of table "pp_nn_1", marked NOT VALID, is incompatible with a primary key.
+HINT: You might need to validate it using ALTER TABLE ... VALIDATE CONSTRAINT.
+DROP TABLE pp_nn;
+-- same as above, but the constraint is NO INHERIT
+CREATE TABLE pp_nn (a int) PARTITION BY HASH (a);
+CREATE TABLE pp_nn_1 PARTITION OF pp_nn FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+ALTER TABLE pp_nn_1 ADD CONSTRAINT nn NOT NULL a NO INHERIT;
+ALTER TABLE ONLY pp_nn ADD PRIMARY KEY (a);
+ERROR: cannot create primary key on column "a"
+DETAIL: The constraint "nn" on column "a" of table "pp_nn_1", marked NO INHERIT, is incompatible with a primary key.
+HINT: You might need to make the existing constraint inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
+DROP TABLE pp_nn;
-- Create table with NOT NULL INVALID constraint, for pg_upgrade.
CREATE TABLE notnull_tbl1_upg (a int, b int);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 1), (NULL, 2), (300, 3);