From de8a1b97f8af214004ebb1b6227f84e7e3ddbd7e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 27 Apr 2022 20:41:49 +0200 Subject: [PATCH v3 2/3] Rebase of catalog-notnull-6, minus psql \d+ changes --- doc/src/sgml/catalogs.sgml | 1 + doc/src/sgml/ref/create_table.sgml | 1 + src/backend/catalog/heap.c | 481 +++++-- src/backend/catalog/pg_constraint.c | 101 ++ src/backend/commands/tablecmds.c | 1126 ++++++++++++----- src/backend/nodes/outfuncs.c | 3 + src/backend/nodes/readfuncs.c | 7 +- src/backend/optimizer/util/plancat.c | 2 + src/backend/parser/gram.y | 13 + src/backend/parser/parse_utilcmd.c | 212 +++- src/backend/utils/adt/ruleutils.c | 12 + src/include/catalog/heap.h | 5 +- src/include/catalog/pg_constraint.h | 11 +- src/include/commands/tablecmds.h | 2 + src/include/nodes/parsenodes.h | 15 +- .../test_ddl_deparse/expected/alter_table.out | 18 +- .../expected/create_table.out | 25 +- .../test_ddl_deparse/test_ddl_deparse.c | 3 + src/test/regress/expected/alter_table.out | 18 +- src/test/regress/expected/cluster.out | 7 +- src/test/regress/expected/constraints.out | 91 ++ src/test/regress/expected/create_table.out | 27 +- src/test/regress/expected/domain.out | 8 + src/test/regress/expected/event_trigger.out | 2 + src/test/regress/expected/foreign_data.out | 11 +- src/test/regress/expected/foreign_key.out | 16 +- src/test/regress/expected/indexing.out | 41 +- src/test/regress/expected/inherit.out | 382 +++++- .../regress/expected/replica_identity.out | 13 + src/test/regress/sql/alter_table.sql | 2 +- src/test/regress/sql/constraints.sql | 33 + src/test/regress/sql/create_table.sql | 6 +- src/test/regress/sql/domain.sql | 7 + src/test/regress/sql/indexing.sql | 8 +- src/test/regress/sql/inherit.sql | 182 ++- src/test/regress/sql/replica_identity.sql | 12 + 36 files changed, 2365 insertions(+), 539 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index c1e4048054..739ec56b3a 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -2543,6 +2543,7 @@ SCRAM-SHA-256$<iteration count>:&l c = check constraint, f = foreign key constraint, + n = not null constraint, p = primary key constraint, u = unique constraint, t = constraint trigger, diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index a03dee4afe..23616c2f5f 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -77,6 +77,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI [ CONSTRAINT constraint_name ] { CHECK ( expression ) [ NO INHERIT ] | + NOT NULL column_name | UNIQUE [ NULLS [ NOT ] DISTINCT ] ( column_name [, ... ] ) index_parameters | PRIMARY KEY ( column_name [, ... ] ) index_parameters | EXCLUDE [ USING index_method ] ( exclude_element WITH operator [, ... ] ) index_parameters [ WHERE ( predicate ) ] | diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 4f006820b8..aa56151c3e 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2160,6 +2160,57 @@ StoreRelCheck(Relation rel, const char *ccname, Node *expr, return constrOid; } +/* + * Store a NOT NULL constraint for the given relation + * + * The OID of the new constraint is returned. + */ +static Oid +StoreRelNotNull(Relation rel, const char *nnname, AttrNumber attnum, + bool is_validated, bool is_local, bool inhcount, + bool is_no_inherit) +{ + int16 attNos; + Oid constrOid; + + /* We only ever store one column per constraint */ + attNos = attnum; + + constrOid = + CreateConstraintEntry(nnname, + RelationGetNamespace(rel), + CONSTRAINT_NOTNULL, + false, + false, + is_validated, + InvalidOid, + RelationGetRelid(rel), + &attNos, + 1, + 1, + InvalidOid, /* not a domain constraint */ + InvalidOid, /* no associated index */ + InvalidOid, /* Foreign key fields */ + NULL, + NULL, + NULL, + NULL, + 0, + ' ', + ' ', + NULL, + 0, + ' ', + NULL, /* not an exclusion constraint */ + NULL, + NULL, + is_local, + inhcount, + is_no_inherit, + false); + return constrOid; +} + /* * Store defaults and constraints (passed as a list of CookedConstraint). * @@ -2204,6 +2255,14 @@ StoreConstraints(Relation rel, List *cooked_constraints, bool is_internal) is_internal); numchecks++; break; + + case CONSTR_NOTNULL: + con->conoid = + StoreRelNotNull(rel, con->name, con->attnum, + !con->skip_validation, con->is_local, + con->inhcount, con->is_no_inherit); + break; + default: elog(ERROR, "unrecognized constraint type: %d", (int) con->contype); @@ -2259,6 +2318,7 @@ AddRelationNewConstraints(Relation rel, ParseNamespaceItem *nsitem; int numchecks; List *checknames; + List *nnnames; ListCell *cell; Node *expr; CookedConstraint *cooked; @@ -2344,130 +2404,179 @@ AddRelationNewConstraints(Relation rel, */ numchecks = numoldchecks; checknames = NIL; + nnnames = NIL; foreach(cell, newConstraints) { Constraint *cdef = (Constraint *) lfirst(cell); - char *ccname; Oid constrOid; - if (cdef->contype != CONSTR_CHECK) - continue; - - if (cdef->raw_expr != NULL) + if (cdef->contype == CONSTR_CHECK) { - Assert(cdef->cooked_expr == NULL); + char *ccname; /* - * Transform raw parsetree to executable expression, and verify - * it's valid as a CHECK constraint. + * XXX Should we detect the case with CHECK (foo IS NOT NULL) and + * handle it as a NOT NULL constraint? */ - expr = cookConstraint(pstate, cdef->raw_expr, - RelationGetRelationName(rel)); - } - else - { - Assert(cdef->cooked_expr != NULL); - /* - * Here, we assume the parser will only pass us valid CHECK - * expressions, so we do no particular checking. - */ - expr = stringToNode(cdef->cooked_expr); - } - - /* - * Check name uniqueness, or generate a name if none was given. - */ - if (cdef->conname != NULL) - { - ListCell *cell2; - - ccname = cdef->conname; - /* Check against other new constraints */ - /* Needed because we don't do CommandCounterIncrement in loop */ - foreach(cell2, checknames) + if (cdef->raw_expr != NULL) { - if (strcmp((char *) lfirst(cell2), ccname) == 0) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("check constraint \"%s\" already exists", - ccname))); + Assert(cdef->cooked_expr == NULL); + + /* + * Transform raw parsetree to executable expression, and + * verify it's valid as a CHECK constraint. + */ + expr = cookConstraint(pstate, cdef->raw_expr, + RelationGetRelationName(rel)); + } + else + { + Assert(cdef->cooked_expr != NULL); + + /* + * Here, we assume the parser will only pass us valid CHECK + * expressions, so we do no particular checking. + */ + expr = stringToNode(cdef->cooked_expr); } - /* save name for future checks */ - checknames = lappend(checknames, ccname); - /* - * Check against pre-existing constraints. If we are allowed to - * merge with an existing constraint, there's no more to do here. - * (We omit the duplicate constraint from the result, which is - * what ATAddCheckConstraint wants.) + * Check name uniqueness, or generate a name if none was given. */ - if (MergeWithExistingConstraint(rel, ccname, expr, - allow_merge, is_local, - cdef->initially_valid, - cdef->is_no_inherit)) - continue; - } - else - { - /* - * When generating a name, we want to create "tab_col_check" for a - * column constraint and "tab_check" for a table constraint. We - * no longer have any info about the syntactic positioning of the - * constraint phrase, so we approximate this by seeing whether the - * expression references more than one column. (If the user - * played by the rules, the result is the same...) - * - * Note: pull_var_clause() doesn't descend into sublinks, but we - * eliminated those above; and anyway this only needs to be an - * approximate answer. - */ - List *vars; - char *colname; + if (cdef->conname != NULL) + { + ListCell *cell2; - vars = pull_var_clause(expr, 0); + ccname = cdef->conname; + /* Check against other new constraints */ + /* Needed because we don't do CommandCounterIncrement in loop */ + foreach(cell2, checknames) + { + if (strcmp((char *) lfirst(cell2), ccname) == 0) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("check constraint \"%s\" already exists", + ccname))); + } - /* eliminate duplicates */ - vars = list_union(NIL, vars); + /* save name for future checks */ + checknames = lappend(checknames, ccname); - if (list_length(vars) == 1) - colname = get_attname(RelationGetRelid(rel), - ((Var *) linitial(vars))->varattno, - true); + /* + * Check against pre-existing constraints. If we are allowed + * to merge with an existing constraint, there's no more to do + * here. (We omit the duplicate constraint from the result, + * which is what ATAddCheckConstraint wants.) + */ + if (MergeWithExistingConstraint(rel, ccname, expr, + allow_merge, is_local, + cdef->initially_valid, + cdef->is_no_inherit)) + continue; + } else - colname = NULL; + { + /* + * When generating a name, we want to create "tab_col_check" + * for a column constraint and "tab_check" for a table + * constraint. We no longer have any info about the syntactic + * positioning of the constraint phrase, so we approximate + * this by seeing whether the expression references more than + * one column. (If the user played by the rules, the result + * is the same...) + * + * Note: pull_var_clause() doesn't descend into sublinks, but + * we eliminated those above; and anyway this only needs to be + * an approximate answer. + */ + List *vars; + char *colname; - ccname = ChooseConstraintName(RelationGetRelationName(rel), - colname, - "check", - RelationGetNamespace(rel), - checknames); + vars = pull_var_clause(expr, 0); - /* save name for future checks */ - checknames = lappend(checknames, ccname); + /* eliminate duplicates */ + vars = list_union(NIL, vars); + + if (list_length(vars) == 1) + colname = get_attname(RelationGetRelid(rel), + ((Var *) linitial(vars))->varattno, + true); + else + colname = NULL; + + ccname = ChooseConstraintName(RelationGetRelationName(rel), + colname, + "check", + RelationGetNamespace(rel), + checknames); + + /* save name for future checks */ + checknames = lappend(checknames, ccname); + } + + /* + * OK, store it. + */ + constrOid = + StoreRelCheck(rel, ccname, expr, cdef->initially_valid, is_local, + is_local ? 0 : 1, cdef->is_no_inherit, is_internal); + + numchecks++; + + cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); + cooked->contype = CONSTR_CHECK; + cooked->conoid = constrOid; + cooked->name = ccname; + cooked->attnum = 0; + cooked->expr = expr; + cooked->skip_validation = cdef->skip_validation; + cooked->is_local = is_local; + cooked->inhcount = is_local ? 0 : 1; + cooked->is_no_inherit = cdef->is_no_inherit; + cookedConstraints = lappend(cookedConstraints, cooked); } + else if (cdef->contype == CONSTR_NOTNULL) + { + CookedConstraint *nncooked; + AttrNumber colnum; + char *nnname; - /* - * OK, store it. - */ - constrOid = - StoreRelCheck(rel, ccname, expr, cdef->initially_valid, is_local, - is_local ? 0 : 1, cdef->is_no_inherit, is_internal); + colnum = get_attnum(RelationGetRelid(rel), + cdef->colname); + if (colnum == InvalidAttrNumber) + elog(ERROR, "invalid column name \"%s\"", cdef->colname); - numchecks++; + if (cdef->conname) + nnname = cdef->conname; /* verify clash? */ + else + nnname = ChooseConstraintName(RelationGetRelationName(rel), + cdef->colname, + "not_null", + RelationGetNamespace(rel), + nnnames); + nnnames = lappend(nnnames, nnname); - cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); - cooked->contype = CONSTR_CHECK; - cooked->conoid = constrOid; - cooked->name = ccname; - cooked->attnum = 0; - cooked->expr = expr; - cooked->skip_validation = cdef->skip_validation; - cooked->is_local = is_local; - cooked->inhcount = is_local ? 0 : 1; - cooked->is_no_inherit = cdef->is_no_inherit; - cookedConstraints = lappend(cookedConstraints, cooked); + constrOid = + StoreRelNotNull(rel, nnname, colnum, + cdef->initially_valid, + is_local, + is_local ? 0 : 1, + cdef->is_no_inherit); + + nncooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); + nncooked->contype = CONSTR_NOTNULL; + nncooked->conoid = constrOid; + nncooked->name = nnname; + nncooked->attnum = colnum; + nncooked->expr = NULL; + nncooked->skip_validation = cdef->skip_validation; + nncooked->is_local = is_local; + nncooked->inhcount = is_local ? 0 : 1; + nncooked->is_no_inherit = cdef->is_no_inherit; + + cookedConstraints = lappend(cookedConstraints, nncooked); + } } /* @@ -2632,6 +2741,180 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr, return found; } +/* list_sort comparator to sort CookedConstraint by attnum */ +static int +list_cookedconstr_attnum_cmp(const ListCell *p1, const ListCell *p2) +{ + AttrNumber v1 = ((CookedConstraint *) lfirst(p1))->attnum; + AttrNumber v2 = ((CookedConstraint *) lfirst(p2))->attnum; + + if (v1 < v2) + return -1; + if (v1 > v2) + return 1; + return 0; +} + +/* + * Create the NOT NULL constraint for the relation + * + * These come from two sources: the 'constraints' list (of Constraint) is + * specified directly by the user; the 'old_notnulls' list (of + * CookedConstraint) comes from inheritance. We create one constraint + * for each column, giving priority to user-specified ones, and setting + * inhcount according to how many parents cause each column to get a + * NOT NULL constraint. If a user-specified name clashes with another + * user-specified name, an error is raised. + * + * Note that inherited constraints have two shapes: those coming from another + * NOT NULL constraint in the parent, which have a name already, and those + * coming from a PRIMARY KEY in the parent, which don't. Any name specified + * in a parent is disregarded in case of a conflict. + */ +void +AddRelationNotNullConstraints(Relation rel, List *constraints, + List *old_notnulls) +{ + List *nnnames = NIL; + List *givennames = NIL; + AttrNumber prev_attnum; + ListCell *lc; + + /* + * First, create all NOT NULLs that are directly specified by the user. + * Note that inheritance might have given us another source for each, so + * we must scan the old_notnulls list and increment inhcount for each + * element with identical attnum. We delete from there any element that + * we process. + */ + foreach(lc, constraints) + { + Constraint *constr = lfirst_node(Constraint, lc); + AttrNumber attnum; + char *conname; + bool is_local = true; + int inhcount = 0; + ListCell *lc2; + + attnum = get_attnum(RelationGetRelid(rel), constr->colname); + + foreach(lc2, old_notnulls) + { + CookedConstraint *old = (CookedConstraint *) lfirst(lc2); + + if (old->attnum == attnum) + { + inhcount++; + old_notnulls = foreach_delete_current(old_notnulls, lc2); + } + } + + /* + * Determine a constraint name, which may have been specified by the + * user, or raise an error if a conflict exists with another + * user-specified name. + */ + if (constr->conname) + { + foreach(lc2, givennames) + { + if (strcmp(lfirst(lc2), conname) == 0) + ereport(ERROR, + errmsg("constraint name \"%s\" is already in use in relation \"%s\"", + constr->conname, + RelationGetRelationName(rel))); + } + + conname = constr->conname; + givennames = lappend(givennames, conname); + } + else + conname = ChooseConstraintName(RelationGetRelationName(rel), + get_attname(RelationGetRelid(rel), + attnum, false), + "not_null", + RelationGetNamespace(rel), + nnnames); + nnnames = lappend(nnnames, conname); + + StoreRelNotNull(rel, conname, + attnum, true, is_local, + inhcount, false); + } + + /* + * If any column remains in the additional_notnulls list, we must create a + * NOT NULL constraint marked not-local. Because multiple parents could + * specify a NOT NULL for the same column, we must count how many there + * are and set inhcount accordingly. Note that unlike the loop above, we + * cannot delete elements in the inner foreach here! So we keep track of + * the element we just saw and skip any that are identical. This requires + * the list to be sorted! Most of the time, this list will be empty. + */ + list_sort(old_notnulls, list_cookedconstr_attnum_cmp); + prev_attnum = InvalidAttrNumber; + foreach(lc, old_notnulls) + { + CookedConstraint *cooked = (CookedConstraint *) lfirst(lc); + char *conname = NULL; + int inhcount = 1; + ListCell *lc2; + + if (cooked->attnum == prev_attnum) + continue; + + /* We just preserve the first constraint name we come across, if any */ + if (conname == NULL && cooked->name) + conname = cooked->name; + + foreach(lc2, old_notnulls) + { + CookedConstraint *other = (CookedConstraint *) lfirst(lc2); + + if (lc2 == lc) + continue; + + if (other->attnum == cooked->attnum) + { + if (conname == NULL && other->name) + conname = other->name; + + inhcount++; + /* can't delete element here; must skip later */ + } + } + + /* If we got a name, make sure it isn't one we've already used */ + if (conname != NULL) + { + foreach(lc2, nnnames) + { + if (strcmp(lfirst(lc2), conname) == 0) + { + conname = NULL; + break; + } + } + } + + /* and choose a name, if needed */ + if (conname == NULL) + conname = ChooseConstraintName(RelationGetRelationName(rel), + get_attname(RelationGetRelid(rel), + cooked->attnum, false), + "not_null", + RelationGetNamespace(rel), + nnnames); + nnnames = lappend(nnnames, conname); + + StoreRelNotNull(rel, conname, cooked->attnum, true, + false, inhcount, + false); + + prev_attnum = cooked->attnum; + } +} + /* * Update the count of constraints in the relation's pg_class tuple. * diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 7392c72e90..9f26e0fbf2 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -562,6 +562,107 @@ ChooseConstraintName(const char *name1, const char *name2, return conname; } +/* + * Find and return the pg_constraint tuple that implements a validated + * NOT NULL constraint for the given column of the given relation. + * + * XXX This would be easier if we had pg_attribute.notnullconstr with the OID + * of the constraint that implements the NOT NULL constraint for that column. + * I'm not sure it's worth the catalog bloat and de-normalization, however. + */ +HeapTuple +findNotNullConstraintAttnum(Relation rel, AttrNumber attnum) +{ + Relation pg_constraint; + HeapTuple conTup, + retval = NULL; + SysScanDesc scan; + ScanKeyData key; + + pg_constraint = table_open(ConstraintRelationId, AccessShareLock); + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + + while (HeapTupleIsValid(conTup = systable_getnext(scan))) + { + Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(conTup); + AttrNumber conkey; + + /* + * We're looking for a NOTNULL constraint that's marked validated, + * with the column we're looking for as the sole element in conkey. + */ + if (con->contype != CONSTRAINT_NOTNULL) + continue; + if (!con->convalidated) + continue; + + conkey = extractNotNullColumn(conTup); + if (conkey != attnum) + continue; + + /* Found it */ + retval = heap_copytuple(conTup); + break; + } + + systable_endscan(scan); + table_close(pg_constraint, AccessShareLock); + + return retval; +} + +/* + * Find and return the pg_constraint tuple that implements a validated + * NOT NULL constraint for the given column of the given relation. + */ +HeapTuple +findNotNullConstraint(Relation rel, const char *colname) +{ + AttrNumber attnum = get_attnum(RelationGetRelid(rel), colname); + + return findNotNullConstraintAttnum(rel, attnum); +} + +/* + * Given a pg_constraint tuple for a NOT NULL constraint, return the column + * number it is for. + */ +AttrNumber +extractNotNullColumn(HeapTuple constrTup) +{ + Form_pg_constraint conForm = (Form_pg_constraint) GETSTRUCT(constrTup); + AttrNumber colnum; + Datum adatum; + ArrayType *arr; + bool isnull; + + /* only tuples for CHECK constraints should be given */ + Assert(conForm->contype == CONSTRAINT_NOTNULL); + + adatum = SysCacheGetAttr(CONSTROID, constrTup, + Anum_pg_constraint_conkey, &isnull); + if (isnull) + elog(ERROR, "null conkey for NOT NULL constraint %u", conForm->oid); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID || + ARR_DIMS(arr)[0] != 1) + elog(ERROR, "conkey is not a 1-D smallint array"); + + memcpy(&colnum, ARR_DATA_PTR(arr), sizeof(AttrNumber)); + + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + + return colnum; +} + /* * Delete a single constraint record. */ diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 62d9917ca3..da2a26a2d2 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -202,7 +202,8 @@ typedef struct AlteredTableInfo typedef struct NewConstraint { char *name; /* Constraint name, or NULL if none */ - ConstrType contype; /* CHECK or FOREIGN */ + ConstrType contype; /* CHECK, NOTNULL, FOREIGN */ + AttrNumber attnum; /* column number, if NOTNULL */ Oid refrelid; /* PK rel, if FOREIGN */ Oid refindid; /* OID of PK's index, if FOREIGN */ Oid conid; /* OID of pg_constraint entry, if FOREIGN */ @@ -349,7 +350,8 @@ static void truncate_check_activity(Relation rel); static void RangeVarCallbackForTruncate(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); static List *MergeAttributes(List *schema, List *supers, char relpersistence, - bool is_partition, List **supconstr); + bool is_partition, List **supconstr, + List **additional_notnulls); static bool MergeCheckConstraint(List *constraints, char *name, Node *expr); static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel); static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel); @@ -430,14 +432,14 @@ static bool check_for_column_name_collision(Relation rel, const char *colname, bool if_not_exists); static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid); static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid); -static void ATPrepDropNotNull(Relation rel, bool recurse, bool recursing); -static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode); -static void ATPrepSetNotNull(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode, - AlterTableUtilityContext *context); -static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, - const char *colName, LOCKMODE lockmode); +static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, bool recurse, + LOCKMODE lockmode); +static ObjectAddress ATExecSetNotNull(List **wqueue, AlteredTableInfo *tab, + Relation rel, char *constrname, char *colName, + bool recurse, bool recursing, + List **readyRels, LOCKMODE lockmode); +static void ATExecSetAttNotNull(AlteredTableInfo *tab, Relation rel, + const char *colName, LOCKMODE lockmode); static void ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel, const char *colName, LOCKMODE lockmode); static bool NotNullImpliedByRelConstraints(Relation rel, Form_pg_attribute attr); @@ -540,6 +542,10 @@ static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, bool missing_ok, LOCKMODE lockmode); +static ObjectAddress dropconstraint_internal(Relation rel, + HeapTuple constraintTup, DropBehavior behavior, + bool recurse, bool recursing, + bool missing_ok, LOCKMODE lockmode); static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, @@ -633,6 +639,7 @@ static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, Relation partitionTbl); +static void verifyPartitionIndexNotNull(IndexInfo *iinfo, Relation partIdx); static List *GetParentedForeignKeyRefs(Relation partition); static void ATDetachCheckNoForeignKeyRefs(Relation partition); static char GetAttributeCompression(Oid atttypid, char *compression); @@ -670,6 +677,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, TupleDesc descriptor; List *inheritOids; List *old_constraints; + List *old_notnulls; List *rawDefaults; List *cookedDefaults; Datum reloptions; @@ -861,12 +869,13 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, MergeAttributes(stmt->tableElts, inheritOids, stmt->relation->relpersistence, stmt->partbound != NULL, - &old_constraints); + &old_constraints, &old_notnulls); /* * Create a tuple descriptor from the relation schema. Note that this - * deals with column names, types, and NOT NULL constraints, but not - * default values or CHECK constraints; we handle those below. + * deals with column names, types, and in-descriptor NOT NULL flags, but + * not default values, NOT NULL or CHECK constraints; we handle those + * below. */ descriptor = BuildDescForRelation(stmt->tableElts); @@ -1248,6 +1257,14 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, AddRelationNewConstraints(rel, NIL, stmt->constraints, true, true, false, queryString); + /* + * Finally, merge the NOT NULL constraints that are directly declared with + * those that come from parent relations (making sure to count inheritance + * appropriately for each), and create them. + */ + AddRelationNotNullConstraints(rel, stmt->nnconstraints, + old_notnulls); + ObjectAddressSet(address, RelationRelationId, relationId); /* @@ -2281,6 +2298,8 @@ storage_name(char c) * Output arguments: * 'supconstr' receives a list of constraints belonging to the parents, * updated as necessary to be valid for the child. + * 'nnconstraints' receives a list of CookedConstraints that corresponds to + * constraints coming from inheritance parents. * * Return value: * Completed schema list. @@ -2311,7 +2330,10 @@ storage_name(char c) * * Constraints (including NOT NULL constraints) for the child table * are the union of all relevant constraints, from both the child schema - * and parent tables. + * and parent tables. In addition, in legacy inheritance, each column that + * appears in a primary key in any of the parents also gets a NOT NULL + * constraint (partitioning doesn't need this, because the PK itself gets + * inherited.) * * The default value for a child column is defined as: * (1) If the child schema specifies a default, that value is used. @@ -2330,10 +2352,11 @@ storage_name(char c) */ static List * MergeAttributes(List *schema, List *supers, char relpersistence, - bool is_partition, List **supconstr) + bool is_partition, List **supconstr, List **supnotnulls) { List *inhSchema = NIL; List *constraints = NIL; + List *nnconstraints = NIL; bool have_bogus_defaults = false; int child_attno; static Node bogus_marker = {0}; /* marks conflicting defaults */ @@ -2445,9 +2468,11 @@ MergeAttributes(List *schema, List *supers, char relpersistence, AttrMap *newattmap; List *inherited_defaults; List *cols_with_defaults; + List *nnconstrs; AttrNumber parent_attno; ListCell *lc1; ListCell *lc2; + Bitmapset *pkattrs; /* caller already got lock */ relation = table_open(parent, NoLock); @@ -2536,6 +2561,16 @@ MergeAttributes(List *schema, List *supers, char relpersistence, /* We can't process inherited defaults until newattmap is complete. */ inherited_defaults = cols_with_defaults = NIL; + /* + * All columns that are part of the parent's primary key need to get a + * NOT NULL constraint, if they don't have one already. + */ + if (!is_partition) + pkattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + else + pkattrs = NULL; /* keep compiler quiet */ + for (parent_attno = 1; parent_attno <= tupleDesc->natts; parent_attno++) { @@ -2620,6 +2655,33 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } def->inhcount++; + + /* + * In regular inheritance, columns in the parent's primary key + * get an extra CHECK (NOT NULL) constraint. Partitioning + * doesn't need this, because the PK itself is going to be + * cloned to the partition. + */ + if (!is_partition && + bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber, + pkattrs)) + { + CookedConstraint *nn; + + nn = palloc(sizeof(CookedConstraint)); + nn->contype = CONSTR_NOTNULL; + nn->conoid = InvalidOid; + nn->name = NULL; + nn->attnum = exist_attno; + nn->expr = NULL; + nn->skip_validation = false; + nn->is_local = false; + nn->inhcount = 1; + nn->is_no_inherit = false; + + nnconstraints = lappend(nnconstraints, nn); + } + /* Merge of NOT NULL constraints = OR 'em together */ def->is_not_null |= attribute->attnotnull; /* Default and other constraints are handled below */ @@ -2660,6 +2722,33 @@ MergeAttributes(List *schema, List *supers, char relpersistence, def->compression = NULL; inhSchema = lappend(inhSchema, def); newattmap->attnums[parent_attno - 1] = ++child_attno; + + /* + * In regular inheritance, columns in the parent's primary key + * get an extra NOT NULL constraint. Partitioning doesn't + * need this, because the PK itself is going to be cloned to + * the partition. + */ + if (!is_partition && + bms_is_member(parent_attno - + FirstLowInvalidHeapAttributeNumber, + pkattrs)) + { + CookedConstraint *nn; + + nn = palloc(sizeof(CookedConstraint)); + nn->contype = CONSTR_NOTNULL; + nn->conoid = InvalidOid; + nn->name = NULL; + nn->attnum = newattmap->attnums[parent_attno - 1]; + nn->expr = NULL; + nn->skip_validation = false; + nn->is_local = false; + nn->inhcount = 1; + nn->is_no_inherit = false; + + nnconstraints = lappend(nnconstraints, nn); + } } /* @@ -2804,6 +2893,19 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } } + /* + * Also copy the NOT NULL constraints from this parent. + */ + nnconstrs = RelationGetNotNullConstraints(relation, true); + foreach(lc1, nnconstrs) + { + CookedConstraint *nn = lfirst(lc1); + + nn->attnum = newattmap->attnums[nn->attnum - 1]; + + nnconstraints = lappend(nnconstraints, nn); + } + free_attrmap(newattmap); /* @@ -2994,8 +3096,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence, /* * Now that we have the column definition list for a partition, we can * check whether the columns referenced in the column constraint specs - * actually exist. Also, we merge parent's NOT NULL constraints and - * defaults into each corresponding column definition. + * actually exist. */ if (is_partition) { @@ -3101,6 +3202,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } *supconstr = constraints; + *supnotnulls = nnconstraints; + return schema; } @@ -3148,6 +3251,80 @@ MergeCheckConstraint(List *constraints, char *name, Node *expr) return false; } +/* + * RelationGetNotNullConstraints -- get list of NOT NULL constraints + * + * Caller can request cooked constraints, or raw. + * + * This is seldom needed, so we just scan pg_constraint each time. + */ +List * +RelationGetNotNullConstraints(Relation relation, bool cooked) +{ + List *notnulls = NIL; + Relation constrRel; + HeapTuple htup; + SysScanDesc conscan; + ScanKeyData skey; + + constrRel = table_open(ConstraintRelationId, AccessShareLock); + ScanKeyInit(&skey, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(relation))); + conscan = systable_beginscan(constrRel, ConstraintRelidTypidNameIndexId, true, + NULL, 1, &skey); + + while (HeapTupleIsValid(htup = systable_getnext(conscan))) + { + Form_pg_constraint conForm = (Form_pg_constraint) GETSTRUCT(htup); + AttrNumber colnum; + + if (conForm->contype != CONSTRAINT_NOTNULL) + continue; + + colnum = extractNotNullColumn(htup); + + if (cooked) + { + CookedConstraint *cooked; + + cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint)); + + cooked->contype = CONSTR_NOTNULL; + cooked->name = pstrdup(NameStr(conForm->conname)); + cooked->attnum = colnum; + cooked->expr = NULL; + cooked->skip_validation = false; + cooked->is_local = true; + cooked->inhcount = 0; + cooked->is_no_inherit = conForm->connoinherit; + + notnulls = lappend(notnulls, cooked); + } + else + { + Constraint *constr; + + constr = makeNode(Constraint); + constr->contype = CONSTR_NOTNULL; + constr->conname = pstrdup(NameStr(conForm->conname)); + constr->deferrable = false; + constr->initdeferred = false; + constr->location = -1; + constr->colname = get_attname(RelationGetRelid(relation), + colnum, false); + constr->skip_validation = false; + constr->initially_valid = true; + notnulls = lappend(notnulls, constr); + } + } + + systable_endscan(conscan); + table_close(constrRel, AccessShareLock); + + return notnulls; +} /* * StoreCatalogInheritance @@ -3708,7 +3885,10 @@ rename_constraint_internal(Oid myrelid, constraintOid); con = (Form_pg_constraint) GETSTRUCT(tuple); - if (myrelid && con->contype == CONSTRAINT_CHECK && !con->connoinherit) + if (myrelid && + (con->contype == CONSTRAINT_CHECK || + con->contype == CONSTRAINT_NOTNULL) && + !con->connoinherit) { if (recurse) { @@ -4293,6 +4473,7 @@ AlterTableGetLockLevel(List *cmds) case AT_AddIndexConstraint: case AT_ReplicaIdentity: case AT_SetNotNull: + case AT_SetAttNotNull: case AT_EnableRowSecurity: case AT_DisableRowSecurity: case AT_ForceRowSecurity: @@ -4591,15 +4772,23 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); - ATPrepDropNotNull(rel, recurse, recursing); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); + /* Set up recursion for phase 2; no other prep needed */ + if (recurse) + cmd->recurse = true; pass = AT_PASS_DROP; break; case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); /* Need command-specific recursion decision */ - ATPrepSetNotNull(wqueue, rel, cmd, recurse, recursing, - lockmode, context); + if (recurse) + cmd->recurse = true; + pass = AT_PASS_COL_ATTRS; + break; + case AT_SetAttNotNull: /* set pg_attribute.attnotnull without adding + * a constraint */ + ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE); + /* Need command-specific recursion decision */ + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); pass = AT_PASS_COL_ATTRS; break; case AT_CheckNotNull: /* check column is already marked NOT NULL */ @@ -4984,10 +5173,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok, lockmode); break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ - address = ATExecDropNotNull(rel, cmd->name, lockmode); + address = ATExecDropNotNull(rel, cmd->name, cmd->recurse, lockmode); break; case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */ - address = ATExecSetNotNull(tab, rel, cmd->name, lockmode); + address = ATExecSetNotNull(wqueue, tab, rel, NULL, cmd->name, + cmd->recurse, false, NULL, lockmode); + break; + case AT_SetAttNotNull: /* set pg_attribute.attnotnull */ + ATExecSetAttNotNull(tab, rel, cmd->name, lockmode); break; case AT_CheckNotNull: /* check column is already marked NOT NULL */ ATExecCheckNotNull(tab, rel, cmd->name, lockmode); @@ -5326,11 +5519,8 @@ ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, */ switch (cmd2->subtype) { - case AT_SetNotNull: - /* Need command-specific recursion decision */ - ATPrepSetNotNull(wqueue, rel, cmd2, - recurse, false, - lockmode, context); + case AT_SetAttNotNull: + ATSimpleRecursion(wqueue, rel, cmd2, recurse, lockmode, context); pass = AT_PASS_COL_ATTRS; break; case AT_AddIndex: @@ -5698,6 +5888,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) TupleDesc oldTupDesc; TupleDesc newTupDesc; bool needscan = false; + bool verify_new_notnull = false; List *notnull_attrs; int i; ListCell *l; @@ -5758,6 +5949,12 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) case CONSTR_FOREIGN: /* Nothing to do here */ break; + case CONSTR_NOTNULL: + if (!NotNullImpliedByRelConstraints(oldrel, + TupleDescAttr(oldTupDesc, + con->attnum - 1))) + verify_new_notnull = true; + break; default: elog(ERROR, "unrecognized constraint type: %d", (int) con->contype); @@ -5780,7 +5977,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) } notnull_attrs = NIL; - if (newrel || tab->verify_new_notnull) + if (newrel || tab->verify_new_notnull || verify_new_notnull) { /* * If we are rebuilding the tuples OR if we added any new but not @@ -6006,6 +6203,7 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) RelationGetRelationName(oldrel)), errtableconstraint(oldrel, con->name))); break; + case CONSTR_NOTNULL: case CONSTR_FOREIGN: /* Nothing to do here */ break; @@ -6114,6 +6312,8 @@ alter_table_type_to_string(AlterTableType cmdtype) return "ALTER COLUMN ... DROP NOT NULL"; case AT_SetNotNull: return "ALTER COLUMN ... SET NOT NULL"; + case AT_SetAttNotNull: + return NULL; /* not real grammar */ case AT_DropExpression: return "ALTER COLUMN ... DROP EXPRESSION"; case AT_CheckNotNull: @@ -6673,8 +6873,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing, */ static ObjectAddress ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, - AlterTableCmd **cmd, - bool recurse, bool recursing, + AlterTableCmd **cmd, bool recurse, bool recursing, LOCKMODE lockmode, int cur_pass, AlterTableUtilityContext *context) { @@ -7181,41 +7380,20 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid) /* * ALTER TABLE ALTER COLUMN DROP NOT NULL - */ - -static void -ATPrepDropNotNull(Relation rel, bool recurse, bool recursing) -{ - /* - * If the parent is a partitioned table, like check constraints, we do not - * support removing the NOT NULL while partitions exist. - */ - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - { - PartitionDesc partdesc = RelationGetPartitionDesc(rel, true); - - Assert(partdesc != NULL); - if (partdesc->nparts > 0 && !recurse && !recursing) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("cannot remove constraint from only the partitioned table when partitions exist"), - errhint("Do not specify the ONLY keyword."))); - } -} - -/* + * * Return the address of the modified column. If the column was already * nullable, InvalidObjectAddress is returned. */ static ObjectAddress -ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) +ATExecDropNotNull(Relation rel, const char *colName, bool recurse, + LOCKMODE lockmode) { HeapTuple tuple; + HeapTuple conTup; + Form_pg_constraint conForm; Form_pg_attribute attTup; AttrNumber attnum; Relation attr_rel; - List *indexoidlist; - ListCell *indexoidscan; ObjectAddress address; /* @@ -7231,6 +7409,15 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) colName, RelationGetRelationName(rel)))); attTup = (Form_pg_attribute) GETSTRUCT(tuple); attnum = attTup->attnum; + ObjectAddressSubSet(address, RelationRelationId, + RelationGetRelid(rel), attnum); + + /* If the column is already nullable there's nothing to do. */ + if (!attTup->attnotnull) + { + table_close(attr_rel, RowExclusiveLock); + return InvalidObjectAddress; + } /* Prevent them from altering a system attribute */ if (attnum <= 0) @@ -7246,68 +7433,43 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) colName, RelationGetRelationName(rel)))); /* - * Check that the attribute is not in a primary key or in an index used as - * a replica identity. - * - * Note: we'll throw error even if the pkey index is not valid. + * It's not OK to remove a constraint only for the parent and leave it in + * the children, so disallow that. */ - - /* Loop over all indexes on the relation */ - indexoidlist = RelationGetIndexList(rel); - - foreach(indexoidscan, indexoidlist) + if (!recurse) { - Oid indexoid = lfirst_oid(indexoidscan); - HeapTuple indexTuple; - Form_pg_index indexStruct; - int i; - - indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexoid)); - if (!HeapTupleIsValid(indexTuple)) - elog(ERROR, "cache lookup failed for index %u", indexoid); - indexStruct = (Form_pg_index) GETSTRUCT(indexTuple); - - /* - * If the index is not a primary key or an index used as replica - * identity, skip the check. - */ - if (indexStruct->indisprimary || indexStruct->indisreplident) + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - /* - * Loop over each attribute in the primary key or the index used - * as replica identity and see if it matches the to-be-altered - * attribute. - */ - for (i = 0; i < indexStruct->indnkeyatts; i++) - { - if (indexStruct->indkey.values[i] == attnum) - { - if (indexStruct->indisprimary) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("column \"%s\" is in a primary key", - colName))); - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("column \"%s\" is in index used as replica identity", - colName))); - } - } - } + PartitionDesc partdesc; - ReleaseSysCache(indexTuple); + partdesc = RelationGetPartitionDesc(rel, true); + + if (partdesc->nparts > 0) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot remove constraint from only the partitioned table when partitions exist"), + errhint("Do not specify the ONLY keyword.")); + } + else if (rel->rd_rel->relhassubclass && + find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) + { + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("NOT NULL constraint on column \"%s\" must be removed in child tables too", + colName), + errhint("Do not specify the ONLY keyword.")); + } } - list_free(indexoidlist); - - /* If rel is partition, shouldn't drop NOT NULL if parent has the same */ + /* + * If rel is partition, shouldn't drop NOT NULL if parent has the same. + */ if (rel->rd_rel->relispartition) { - Oid parentId = get_partition_parent(RelationGetRelid(rel), false); - Relation parent = table_open(parentId, AccessShareLock); - TupleDesc tupDesc = RelationGetDescr(parent); - AttrNumber parent_attnum; + Oid parentId = get_partition_parent(RelationGetRelid(rel), false); + Relation parent = table_open(parentId, AccessShareLock); + TupleDesc tupDesc = RelationGetDescr(parent); + AttrNumber parent_attnum; parent_attnum = get_attnum(parentId, colName); if (TupleDescAttr(tupDesc, parent_attnum - 1)->attnotnull) @@ -7319,22 +7481,41 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) } /* - * Okay, actually perform the catalog change ... if needed + * Find the constraint that makes this column NOT NULL. */ - if (attTup->attnotnull) + conTup = findNotNullConstraint(rel, colName); + if (conTup == NULL) { - attTup->attnotnull = false; + Bitmapset *pkcols; - CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple); + /* + * There's no NOT NULL constraint, so throw an error. If the column + * is in a primary key, we can throw a specific error. Otherwise, + * this is unexpected. + */ + pkcols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_PRIMARY_KEY); + if (bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber, + pkcols)) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("column \"%s\" is in a primary key", colName)); - ObjectAddressSubSet(address, RelationRelationId, - RelationGetRelid(rel), attnum); + /* this shouldn't happen */ + elog(ERROR, "no NOT NULL constraint found to drop"); } - else - address = InvalidObjectAddress; - InvokeObjectPostAlterHook(RelationRelationId, - RelationGetRelid(rel), attnum); + conForm = (Form_pg_constraint) GETSTRUCT(conTup); + + if (conForm->coninhcount > 0) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot drop inherited constraint \"%s\" of relation \"%s\"", + NameStr(conForm->conname), RelationGetRelationName(rel))); + + dropconstraint_internal(rel, conTup, DROP_RESTRICT, recurse, false, + false, lockmode); + + heap_freetuple(conTup); table_close(attr_rel, RowExclusiveLock); @@ -7343,101 +7524,62 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) /* * ALTER TABLE ALTER COLUMN SET NOT NULL - */ - -static void -ATPrepSetNotNull(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode, AlterTableUtilityContext *context) -{ - /* - * If we're already recursing, there's nothing to do; the topmost - * invocation of ATSimpleRecursion already visited all children. - */ - if (recursing) - return; - - /* - * If the target column is already marked NOT NULL, we can skip recursing - * to children, because their columns should already be marked NOT NULL as - * well. But there's no point in checking here unless the relation has - * some children; else we can just wait till execution to check. (If it - * does have children, however, this can save taking per-child locks - * unnecessarily. This greatly improves concurrency in some parallel - * restore scenarios.) - * - * Unfortunately, we can only apply this optimization to partitioned - * tables, because traditional inheritance doesn't enforce that child - * columns be NOT NULL when their parent is. (That's a bug that should - * get fixed someday.) - */ - if (rel->rd_rel->relhassubclass && - rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - { - HeapTuple tuple; - bool attnotnull; - - tuple = SearchSysCacheAttName(RelationGetRelid(rel), cmd->name); - - /* Might as well throw the error now, if name is bad */ - if (!HeapTupleIsValid(tuple)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_COLUMN), - errmsg("column \"%s\" of relation \"%s\" does not exist", - cmd->name, RelationGetRelationName(rel)))); - - attnotnull = ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull; - ReleaseSysCache(tuple); - if (attnotnull) - return; - } - - /* - * If we have ALTER TABLE ONLY ... SET NOT NULL on a partitioned table, - * apply ALTER TABLE ... CHECK NOT NULL to every child. Otherwise, use - * normal recursion logic. - */ - if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && - !recurse) - { - AlterTableCmd *newcmd = makeNode(AlterTableCmd); - - newcmd->subtype = AT_CheckNotNull; - newcmd->name = pstrdup(cmd->name); - ATSimpleRecursion(wqueue, rel, newcmd, true, lockmode, context); - } - else - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); -} - -/* - * Return the address of the modified column. If the column was already NOT - * NULL, InvalidObjectAddress is returned. + * + * Add a NOT NULL constraint to a single table and its children. Returns + * the address of the constraint added to the parent relation, if one gets + * added, or InvalidObjectAddress otherwise. + * + * We must recurse to child tables during execution, rather than using + * ALTER TABLE's normal prep-time recursion. The reason is that all the + * constraints *must* be given the same name, else they won't be seen as + * related later. Because the user cannot specify a constraint name in + * this command form, we must scan the hierarchy to choose a good one + * from the beginning, and pass that down to all children. */ static ObjectAddress -ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, - const char *colName, LOCKMODE lockmode) +ATExecSetNotNull(List **wqueue, AlteredTableInfo *tab, Relation rel, + char *conName, char *colName, + bool recurse, bool recursing, List **readyRels, + LOCKMODE lockmode) { HeapTuple tuple; - AttrNumber attnum; Relation attr_rel; + Relation constr_rel; + ScanKeyData skey; + SysScanDesc conscan; + AttrNumber attnum; ObjectAddress address; + Constraint *constraint; + CookedConstraint *ccon; + bool found = false; + List *cooked; + List *ready = NIL; /* - * lookup the attribute + * In cases of multiple inheritance, we might visit the same child more + * than once. In the topmost call, set up a list that we fill with all + * visited relations, to skip those. */ - attr_rel = table_open(AttributeRelationId, RowExclusiveLock); + if (readyRels == NULL) + readyRels = &ready; + if (list_member_oid(*readyRels, RelationGetRelid(rel))) + return InvalidObjectAddress; + *readyRels = lappend_oid(*readyRels, RelationGetRelid(rel)); - tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + /* At top level, permission check was done in ATPrepCmd, else do it */ + if (recursing) + { + ATSimplePermissions(AT_AddConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE); + Assert(conName != NULL); + } - if (!HeapTupleIsValid(tuple)) + attnum = get_attnum(RelationGetRelid(rel), colName); + if (attnum == InvalidAttrNumber) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("column \"%s\" of relation \"%s\" does not exist", colName, RelationGetRelationName(rel)))); - attnum = ((Form_pg_attribute) GETSTRUCT(tuple))->attnum; - /* Prevent them from altering a system attribute */ if (attnum <= 0) ereport(ERROR, @@ -7445,42 +7587,255 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, errmsg("cannot alter system column \"%s\"", colName))); + /* See if there's already a constraint */ + constr_rel = table_open(ConstraintRelationId, RowExclusiveLock); + ScanKeyInit(&skey, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + conscan = systable_beginscan(constr_rel, ConstraintRelidTypidNameIndexId, true, + NULL, 1, &skey); + + while (HeapTupleIsValid(tuple = systable_getnext(conscan))) + { + Form_pg_constraint conForm = (Form_pg_constraint) GETSTRUCT(tuple); + bool changed = false; + HeapTuple copytup; + + if (conForm->contype != CONSTRAINT_NOTNULL) + continue; + + if (extractNotNullColumn(tuple) != attnum) + continue; + + copytup = heap_copytuple(tuple); + + /* + * If we're recursing (that is, we have already determined a + * constraint name) and we find that an appropriate constraint already + * exists, then rename it to the name we want and increment + * coninhcount. + * + * However, there are some problems: 1) if the constraint on the child + * is inherited, then we cannot rename it because another parent + * forces the current name. Throw error. 2) If the target name we + * chose is used by another constraint, it's not possible to rename + * either (this only happens when tables are in different schemas). + * + * If we're not recursing and we do find a matching constraint, then + * we don't need to add another; just set conislocal for it (if not + * already done) and we're done. + */ + if (recursing) + { + Assert(conName != NULL); + if (strcmp(conName, NameStr(conForm->conname)) != 0) + { + if (conForm->coninhcount > 0) + ereport(ERROR, + errmsg("renaming inherited constraint \"%s\" on relation \"%s\" to \"%s\" is not supported", + NameStr(conForm->conname), + RelationGetRelationName(rel), conName), + errhint("Try renaming the constraint on the other parent(s) of relation \"%s\" first.", + RelationGetRelationName(rel))); + + if (ConstraintNameIsUsed(CONSTRAINT_RELATION, + RelationGetRelid(rel), + conName)) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("cannot rename constraint on relation \"%s.%s\" to \"%s\"", + get_namespace_name(rel->rd_rel->relnamespace), + RelationGetRelationName(rel), + conName), + errdetail("Another constraint with that name already exists.")); + + namestrcpy(&(conForm->conname), conName); + } + + conForm->coninhcount++; + changed = true; + } + else if (!conForm->conislocal) + { + conForm->conislocal = true; + changed = true; + } + + if (changed) + { + CatalogTupleUpdate(constr_rel, &tuple->t_self, copytup); + ObjectAddressSet(address, ConstraintRelationId, conForm->oid); + } + + found = true; + break; + } + + systable_endscan(conscan); + table_close(constr_rel, RowExclusiveLock); + + /* If a found a constraint, no need for anything else */ + if (found) + return address; + /* - * Okay, actually perform the catalog change ... if needed + * If we're asked not to recurse, and children exist, raise an error. */ + if (!recurse && + find_inheritance_children(RelationGetRelid(rel), + NoLock) != NIL) + { + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot add constraint to only the partitioned table when partitions exist"), + errhint("Do not specify the ONLY keyword.")); + else + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot add constraint to table with inheritance children"), + errhint("Do not specify the ONLY keyword.")); + } + + /* + * If we are recursing after having already decided on a name, but that + * name is already taken up in this relation, throw an error. This would + * only happen with relations in different schemas, so mention the schema + * in the message. + */ + if (conName && + ConstraintNameIsUsed(CONSTRAINT_RELATION, + RelationGetRelid(rel), + conName)) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("cannot add constraint \"%s\" to relation \"%s.%s\"", + conName, get_namespace_name(rel->rd_rel->relnamespace), + RelationGetRelationName(rel)), + errdetail("Another constraint with that name already exists.")); + + /* + * No constraint exists; we must add one. First determine a name to use, + * if we haven't already. Note that because how ChooseConstraintName + * works, this name won't match any other constraint name in the schema, + * including potentially ones in the children that we need to recurse to, + * so this will necessarily rename any that exist. + */ + if (!recursing) + { + Assert(conName == NULL); + conName = ChooseConstraintName(RelationGetRelationName(rel), + colName, "not_null", + RelationGetNamespace(rel), + NIL); + } + constraint = makeNode(Constraint); + constraint->contype = CONSTR_NOTNULL; + constraint->conname = conName; + constraint->deferrable = false; + constraint->initdeferred = false; + constraint->location = -1; + constraint->parent_oid = InvalidOid; + constraint->colname = colName; + constraint->skip_validation = false; + constraint->initially_valid = true; + + /* and do it */ + cooked = AddRelationNewConstraints(rel, NIL, list_make1(constraint), + false, !recursing, false, NULL); + ccon = linitial(cooked); + ObjectAddressSet(address, ConstraintRelationId, ccon->conoid); + + /* Set pg_attribute.attnotnull, if it isn't set */ + attr_rel = table_open(AttributeRelationId, RowExclusiveLock); + tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failure for attribute \"%s\" of relation %u", + colName, RelationGetRelid(rel)); if (!((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull) { ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull = true; - CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple); - - /* - * Ordinarily phase 3 must ensure that no NULLs exist in columns that - * are set NOT NULL; however, if we can find a constraint which proves - * this then we can skip that. We needn't bother looking if we've - * already found that we must verify some other NOT NULL constraint. - */ - if (!tab->verify_new_notnull && - !NotNullImpliedByRelConstraints(rel, (Form_pg_attribute) GETSTRUCT(tuple))) - { - /* Tell Phase 3 it needs to test the constraint */ - tab->verify_new_notnull = true; - } - - ObjectAddressSubSet(address, RelationRelationId, - RelationGetRelid(rel), attnum); } - else - address = InvalidObjectAddress; - InvokeObjectPostAlterHook(RelationRelationId, - RelationGetRelid(rel), attnum); + /* + * And set up for existing values to be checked, unless another constraint + * already proves this. + */ + if (!NotNullImpliedByRelConstraints(rel, (Form_pg_attribute) GETSTRUCT(tuple))) + tab->verify_new_notnull = true; table_close(attr_rel, RowExclusiveLock); + /* + * Recurse to propagate the constraint to children that don't have one. + * This also renames it in those that do have it. + */ + if (recurse) + { + List *children; + ListCell *lc; + + children = find_inheritance_children(RelationGetRelid(rel), + lockmode); + + foreach(lc, children) + { + AlteredTableInfo *childtab; + Relation childrel; + + childrel = table_open(lfirst_oid(lc), NoLock); + childtab = ATGetQueueEntry(wqueue, childrel); + + ATExecSetNotNull(wqueue, childtab, childrel, + conName, colName, recurse, true, + readyRels, lockmode); + + table_close(childrel, NoLock); + } + } + return address; } +/* + * ALTER TABLE ALTER COLUMN SET ATTNOTNULL + * + * This doesn't exist in the grammar; it's used when creating a + * primary key and the column is not already marked attnotnull. + */ +static void +ATExecSetAttNotNull(AlteredTableInfo *tab, Relation rel, + const char *colName, LOCKMODE lockmode) +{ + HeapTuple tuple; + Form_pg_attribute attForm; + + tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + colName, RelationGetRelationName(rel))); + attForm = (Form_pg_attribute) GETSTRUCT(tuple); + + if (!attForm->attnotnull) + { + Relation attrel = table_open(AttributeRelationId, RowExclusiveLock); + + attForm->attnotnull = true; + CatalogTupleUpdate(attrel, &tuple->t_self, tuple); + + if (!NotNullImpliedByRelConstraints(rel, attForm)) + tab->verify_new_notnull = true; + + table_close(attrel, RowExclusiveLock); + } + + heap_freetuple(tuple); +} + /* * ALTER TABLE ALTER COLUMN CHECK NOT NULL * @@ -8762,13 +9117,14 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Assert(IsA(newConstraint, Constraint)); /* - * Currently, we only expect to see CONSTR_CHECK and CONSTR_FOREIGN nodes - * arriving here (see the preprocessing done in parse_utilcmd.c). Use a - * switch anyway to make it easier to add more code later. + * Currently, we only expect to see CONSTR_CHECK, CONSTR_NOTNULL and + * CONSTR_FOREIGN nodes arriving here (see the preprocessing done in + * parse_utilcmd.c). */ switch (newConstraint->contype) { case CONSTR_CHECK: + case CONSTR_NOTNULL: address = ATAddCheckConstraint(wqueue, tab, rel, newConstraint, recurse, false, is_readd, @@ -8913,6 +9269,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, NewConstraint *newcon; newcon = (NewConstraint *) palloc0(sizeof(NewConstraint)); + newcon->attnum = ccon->attnum; newcon->name = ccon->name; newcon->contype = ccon->contype; newcon->qual = ccon->expr; @@ -11073,6 +11430,10 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, * partitioned tables, so ignoring the recursion bit is okay. */ } + else if (con->contype == CONSTRAINT_NOTNULL) + { + elog(ERROR, "not implemented yet"); + } else if (con->contype == CONSTRAINT_CHECK) { List *children = NIL; @@ -11845,16 +12206,11 @@ ATExecDropConstraint(Relation rel, const char *constrName, bool recurse, bool recursing, bool missing_ok, LOCKMODE lockmode) { - List *children; - ListCell *child; Relation conrel; - Form_pg_constraint con; SysScanDesc scan; ScanKeyData skey[3]; HeapTuple tuple; bool found = false; - bool is_no_inherit_constraint = false; - char contype; /* At top level, permission check was done in ATPrepCmd, else do it */ if (recursing) @@ -11883,47 +12239,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, /* There can be at most one matching row */ if (HeapTupleIsValid(tuple = systable_getnext(scan))) { - ObjectAddress conobj; - - con = (Form_pg_constraint) GETSTRUCT(tuple); - - /* Don't drop inherited constraints */ - if (con->coninhcount > 0 && !recursing) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("cannot drop inherited constraint \"%s\" of relation \"%s\"", - constrName, RelationGetRelationName(rel)))); - - is_no_inherit_constraint = con->connoinherit; - contype = con->contype; - - /* - * If it's a foreign-key constraint, we'd better lock the referenced - * table and check that that's not in use, just as we've already done - * for the constrained table (else we might, eg, be dropping a trigger - * that has unfired events). But we can/must skip that in the - * self-referential case. - */ - if (contype == CONSTRAINT_FOREIGN && - con->confrelid != RelationGetRelid(rel)) - { - Relation frel; - - /* Must match lock taken by RemoveTriggerById: */ - frel = table_open(con->confrelid, AccessExclusiveLock); - CheckTableNotInUse(frel, "ALTER TABLE"); - table_close(frel, NoLock); - } - - /* - * Perform the actual constraint deletion - */ - conobj.classId = ConstraintRelationId; - conobj.objectId = con->oid; - conobj.objectSubId = 0; - - performDeletion(&conobj, behavior, 0); - + dropconstraint_internal(rel, tuple, behavior, recurse, recursing, + missing_ok, lockmode); found = true; } @@ -11932,31 +12249,218 @@ ATExecDropConstraint(Relation rel, const char *constrName, if (!found) { if (!missing_ok) - { ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("constraint \"%s\" of relation \"%s\" does not exist", - constrName, RelationGetRelationName(rel)))); - } + errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("constraint \"%s\" of relation \"%s\" does not exist", + constrName, RelationGetRelationName(rel))); else - { ereport(NOTICE, - (errmsg("constraint \"%s\" of relation \"%s\" does not exist, skipping", - constrName, RelationGetRelationName(rel)))); - table_close(conrel, RowExclusiveLock); - return; + errmsg("constraint \"%s\" of relation \"%s\" does not exist, skipping", + constrName, RelationGetRelationName(rel))); + } + + table_close(conrel, RowExclusiveLock); +} + +/* + * Remove a constraint, using its pg_constraint tuple + * + * Implementation for ALTER TABLE DROP CONSTRAINT and ALTER TABLE ALTER COLUMN + * DROP NOT NULL. + * + * Returns the address of the constraint being removed. + */ +static ObjectAddress +dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior behavior, + bool recurse, bool recursing, bool missing_ok, LOCKMODE lockmode) +{ + Relation conrel; + Form_pg_constraint con; + ObjectAddress conobj; + List *children; + ListCell *child; + bool is_no_inherit_constraint = false; + bool dropping_pk = false; + char *constrName; + List *unconstrained_cols = NIL; + + conrel = table_open(ConstraintRelationId, RowExclusiveLock); + + con = (Form_pg_constraint) GETSTRUCT(constraintTup); + constrName = NameStr(con->conname); + + /* Don't drop inherited constraints */ + if (con->coninhcount > 0 && !recursing) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot drop inherited constraint \"%s\" of relation \"%s\"", + constrName, RelationGetRelationName(rel)))); + + /* + * See if we have a NOT NULL constraint or a PRIMARY KEY. If so, we have + * more checks and actions below, so obtain the list of columns that are + * constrained by the constraint being dropped. + */ + if (con->contype == CONSTRAINT_NOTNULL) + { + AttrNumber colnum = extractNotNullColumn(constraintTup); + + if (colnum != InvalidAttrNumber) + unconstrained_cols = list_make1_int(colnum); + } + else if (con->contype == CONSTRAINT_PRIMARY) + { + Datum adatum; + ArrayType *arr; + int numkeys; + bool isNull; + int16 *attnums; + + dropping_pk = true; + + adatum = heap_getattr(constraintTup, Anum_pg_constraint_conkey, + RelationGetDescr(conrel), &isNull); + if (isNull) + elog(ERROR, "null conkey for constraint %u", con->oid); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + numkeys = ARR_DIMS(arr)[0]; + if (ARR_NDIM(arr) != 1 || + numkeys < 0 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID) + elog(ERROR, "conkey is not a 1-D smallint array"); + attnums = (int16 *) ARR_DATA_PTR(arr); + + for (int i = 0; i < numkeys; i++) + unconstrained_cols = lappend_int(unconstrained_cols, attnums[i]); + } + + is_no_inherit_constraint = con->connoinherit; + + /* + * If it's a foreign-key constraint, we'd better lock the referenced table + * and check that that's not in use, just as we've already done for the + * constrained table (else we might, eg, be dropping a trigger that has + * unfired events). But we can/must skip that in the self-referential + * case. + */ + if (con->contype == CONSTRAINT_FOREIGN && + con->confrelid != RelationGetRelid(rel)) + { + Relation frel; + + /* Must match lock taken by RemoveTriggerById: */ + frel = table_open(con->confrelid, AccessExclusiveLock); + CheckTableNotInUse(frel, "ALTER TABLE"); + table_close(frel, NoLock); + } + + /* + * Perform the actual constraint deletion + */ + ObjectAddressSet(conobj, ConstraintRelationId, con->oid); + performDeletion(&conobj, behavior, 0); + + /* + * If this was a CHECK (col IS NOT NULL) or the primary key, the + * constrained columns must have had pg_attribute.attnotnull set. See if + * we need to reset it, and do so. + */ + if (unconstrained_cols) + { + Relation attrel; + Bitmapset *pkcols; + Bitmapset *ircols; + ListCell *lc; + + /* Make the above deletion visible */ + CommandCounterIncrement(); + + attrel = table_open(AttributeRelationId, RowExclusiveLock); + + /* + * We want to test columns for their presence in the primary key, but + * only if we're not dropping it. + */ + pkcols = dropping_pk ? NULL : + RelationGetIndexAttrBitmap(rel, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + ircols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_IDENTITY_KEY); + + foreach(lc, unconstrained_cols) + { + AttrNumber attnum = lfirst_int(lc); + HeapTuple atttup; + HeapTuple contup; + Form_pg_attribute attForm; + + /* + * Obtain pg_attribute tuple and verify conditions on it. We use + * a copy we can scribble on. + */ + atttup = SearchSysCacheCopyAttNum(RelationGetRelid(rel), attnum); + if (!HeapTupleIsValid(atttup)) + elog(ERROR, "cache lookup failed for column %d", attnum); + attForm = (Form_pg_attribute) GETSTRUCT(atttup); + + /* + * Since the above deletion has been made visible, we can now + * search for any remaining constraints on this column (or these + * columns, in the case we're dropping a multicol primary key.) + * Then, verify whether any further NOT NULL or primary key exist, + * and reset attnotnull if none. + * + * However, if this is a generated identity column, abort the + * whole thing with a specific error message, because the + * constraint is required in that case. + */ + contup = findNotNullConstraintAttnum(rel, attnum); + if (contup || + bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber, + pkcols)) + continue; + + /* + * It's not valid to drop the last NOT NULL constraint for a + * GENERATED AS IDENTITY column. + */ + if (attForm->attidentity) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("column \"%s\" of relation \"%s\" is an identity column", + get_attname(RelationGetRelid(rel), attnum, + false), + RelationGetRelationName(rel))); + + /* + * It's not valid to drop the last NOT NULL constraint for the + * replica identity either. XXX make exception for FULL? + */ + if (bms_is_member(lfirst_int(lc) - FirstLowInvalidHeapAttributeNumber, ircols)) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("column \"%s\" is in index used as replica identity", + get_attname(RelationGetRelid(rel), lfirst_int(lc), false))); + + /* Reset attnotnull */ + if (attForm->attnotnull) + { + attForm->attnotnull = false; + CatalogTupleUpdate(attrel, &atttup->t_self, atttup); + } } + table_close(attrel, RowExclusiveLock); } /* * For partitioned tables, non-CHECK inherited constraints are dropped via * the dependency mechanism, so we're done here. */ - if (contype != CONSTRAINT_CHECK && + if (con->contype != CONSTRAINT_CHECK && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { table_close(conrel, RowExclusiveLock); - return; + return conobj; } /* @@ -11985,7 +12489,10 @@ ATExecDropConstraint(Relation rel, const char *constrName, { Oid childrelid = lfirst_oid(child); Relation childrel; + HeapTuple tuple; HeapTuple copy_tuple; + SysScanDesc scan; + ScanKeyData skey[3]; /* find_inheritance_children already got lock */ childrel = table_open(childrelid, NoLock); @@ -12020,9 +12527,10 @@ ATExecDropConstraint(Relation rel, const char *constrName, con = (Form_pg_constraint) GETSTRUCT(copy_tuple); - /* Right now only CHECK constraints can be inherited */ - if (con->contype != CONSTRAINT_CHECK) - elog(ERROR, "inherited constraint is not a CHECK constraint"); + /* Right now only CHECK and NOT NULL constraints can be inherited */ + if (con->contype != CONSTRAINT_CHECK && + con->contype != CONSTRAINT_NOTNULL) + elog(ERROR, "inherited constraint is not a CHECK or NOT NULL constraint"); if (con->coninhcount <= 0) /* shouldn't happen */ elog(ERROR, "relation %u has non-inherited constraint \"%s\"", @@ -12037,6 +12545,7 @@ ATExecDropConstraint(Relation rel, const char *constrName, if (con->coninhcount == 1 && !con->conislocal) { /* Time to delete this child constraint, too */ + /* XXX can this recurse on itself instead? */ ATExecDropConstraint(childrel, constrName, behavior, true, true, false, lockmode); @@ -12073,6 +12582,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, } table_close(conrel, RowExclusiveLock); + + return conobj; } /* @@ -13394,10 +13905,10 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, NIL, con->conname); } - else if (cmd->subtype == AT_SetNotNull) + else if (cmd->subtype == AT_SetAttNotNull) { /* - * The parser will create AT_SetNotNull subcommands for + * The parser will create AT_AttSetNotNull subcommands for * columns of PRIMARY KEY indexes/constraints, but we need * not do anything with them here, because the columns' * NOT NULL marks will already have been propagated into @@ -15139,6 +15650,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) SysScanDesc parent_scan; ScanKeyData parent_key; HeapTuple parent_tuple; + Oid parent_relid = RelationGetRelid(parent_rel); bool child_is_partition = false; catalog_relation = table_open(ConstraintRelationId, RowExclusiveLock); @@ -15152,7 +15664,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) ScanKeyInit(&parent_key, Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(parent_rel))); + ObjectIdGetDatum(parent_relid)); parent_scan = systable_beginscan(catalog_relation, ConstraintRelidTypidNameIndexId, true, NULL, 1, &parent_key); @@ -15500,7 +16012,8 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) bool match; ListCell *lc; - if (con->contype != CONSTRAINT_CHECK) + if (con->contype != CONSTRAINT_CHECK && + con->contype != CONSTRAINT_NOTNULL) continue; match = false; @@ -18978,6 +19491,13 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) RelationGetRelationName(partIdx)))); } + /* + * If it's a primary key, make sure the columns in the partition are + * NOT NULL. + */ + if (parentIdx->rd_index->indisprimary) + verifyPartitionIndexNotNull(childInfo, partTbl); + /* All good -- do it */ IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx)); if (OidIsValid(constraintOid)) @@ -19114,6 +19634,30 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) } } +/* + * When a primary key index on a partitioned table is to be attached an index + * on a partition, the partition's columns should also be marked NOT NULL. + * Ensure that is the case. + */ +static void +verifyPartitionIndexNotNull(IndexInfo *iinfo, Relation partition) +{ + for (int i = 0; i < iinfo->ii_NumIndexKeyAttrs; i++) + { + Form_pg_attribute att = TupleDescAttr(RelationGetDescr(partition), + iinfo->ii_IndexAttrNumbers[i] - 1); + + if (!att->attnotnull) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("invalid primary key definition"), + errdetail("Column \"%s\" of relation \"%s\" is not marked NOT NULL.", + NameStr(att->attname), + RelationGetRelationName(partition))); + } +} + + /* * Return an OID list of constraints that reference the given relation * that are marked as having a parent constraints. diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index ba00b99249..9b88b4a40a 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -717,6 +717,9 @@ _outConstraint(StringInfo str, const Constraint *node) case CONSTR_NOTNULL: appendStringInfoString(str, "NOT_NULL"); + WRITE_STRING_FIELD(colname); + WRITE_BOOL_FIELD(skip_validation); + WRITE_BOOL_FIELD(initially_valid); break; case CONSTR_DEFAULT: diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index f3629cdfd1..7fd2a5ffae 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -367,10 +367,15 @@ _readConstraint(void) switch (local_node->contype) { case CONSTR_NULL: - case CONSTR_NOTNULL: /* no extra fields */ break; + case CONSTR_NOTNULL: + READ_STRING_FIELD(colname); + READ_BOOL_FIELD(skip_validation); + READ_BOOL_FIELD(initially_valid); + break; + case CONSTR_DEFAULT: READ_NODE_FIELD(raw_expr); READ_STRING_FIELD(cooked_expr); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index d58c4a1078..9809d1a1c7 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -1644,6 +1644,8 @@ relation_excluded_by_constraints(PlannerInfo *root, * Currently, attnotnull constraints must be treated as NO INHERIT unless * this is a partitioned table. In future we might track their * inheritance status more accurately, allowing this to be refined. + * + * XXX do we need/want to change this? */ include_notnull = (!rte->inh || rte->relkind == RELKIND_PARTITIONED_TABLE); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index a0138382a1..553fe74eeb 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4074,6 +4074,19 @@ ConstraintElem: n->initially_valid = !n->skip_validation; $$ = (Node *) n; } + | NOT NULL_P ColId ConstraintAttributeSpec + { + Constraint *n = makeNode(Constraint); + + n->contype = CONSTR_NOTNULL; + n->location = @1; + n->colname = $3; + processCASbits($4, @4, "NOT NULL", + NULL, NULL, NULL, + NULL, yyscanner); + n->initially_valid = !n->skip_validation; + $$ = (Node *) n; + } | UNIQUE opt_unique_null_treatment '(' columnList ')' opt_c_include opt_definition OptConsTableSpace ConstraintAttributeSpec { diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index f9218f48aa..b448d2e8b0 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -80,9 +80,10 @@ typedef struct bool isforeign; /* true if CREATE/ALTER FOREIGN TABLE */ bool isalter; /* true if altering existing table */ List *columns; /* ColumnDef items */ - List *ckconstraints; /* CHECK constraints */ + List *ckconstraints; /* CHECK and NOT NULL constraints */ List *fkconstraints; /* FOREIGN KEY constraints */ List *ixconstraints; /* index-creating constraints */ + List *nnconstraints; /* NOT NULL constraints */ List *likeclauses; /* LIKE clauses that need post-processing */ List *extstats; /* cloned extended statistics */ List *blist; /* "before list" of things to do before @@ -244,6 +245,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) cxt.ckconstraints = NIL; cxt.fkconstraints = NIL; cxt.ixconstraints = NIL; + cxt.nnconstraints = NIL; cxt.likeclauses = NIL; cxt.extstats = NIL; cxt.blist = NIL; @@ -348,6 +350,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) */ stmt->tableElts = cxt.columns; stmt->constraints = cxt.ckconstraints; + stmt->nnconstraints = cxt.nnconstraints; result = lappend(cxt.blist, stmt); result = list_concat(result, cxt.alist); @@ -530,10 +533,11 @@ static void transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) { bool is_serial; - bool saw_nullable; bool saw_default; + bool saw_nullable; bool saw_identity; bool saw_generated; + bool need_notnull = false; ListCell *clist; cxt->columns = lappend(cxt->columns, column); @@ -631,10 +635,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) constraint->cooked_expr = NULL; column->constraints = lappend(column->constraints, constraint); - constraint = makeNode(Constraint); - constraint->contype = CONSTR_NOTNULL; - constraint->location = -1; - column->constraints = lappend(column->constraints, constraint); + /* have a NOT NULL constraint added later */ + need_notnull = true; } /* Process column constraints, if any... */ @@ -652,7 +654,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) switch (constraint->contype) { case CONSTR_NULL: - if (saw_nullable && column->is_not_null) + if ((saw_nullable && column->is_not_null) || need_notnull) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"", @@ -664,15 +666,59 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) break; case CONSTR_NOTNULL: - if (saw_nullable && !column->is_not_null) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"", - column->colname, cxt->relation->relname), - parser_errposition(cxt->pstate, - constraint->location))); - column->is_not_null = true; - saw_nullable = true; + + /* + * For NOT NULL declarations, we need to mark the column as + * not nullable, and set things up to have a CHECK constraint + * created. Also, duplicate NOT NULL declarations are not + * allowed. + */ + if (saw_nullable) + { + if (!column->is_not_null) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"", + column->colname, cxt->relation->relname), + parser_errposition(cxt->pstate, + constraint->location))); + else + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("redundant NOT NULL declarations for column \"%s\" of table \"%s\"", + column->colname, cxt->relation->relname), + parser_errposition(cxt->pstate, + constraint->location)); + } + + /* + * If this is the first time we see this column being marked + * not null, keep track to later add a NOT NULL constraint. + */ + if (!column->is_not_null) + { + Constraint *notnull; + + column->is_not_null = true; + saw_nullable = true; + + notnull = makeNode(Constraint); + notnull->contype = CONSTR_NOTNULL; + notnull->conname = constraint->conname; + notnull->deferrable = false; + notnull->initdeferred = false; + notnull->location = -1; + notnull->parent_oid = InvalidOid; + notnull->colname = column->colname; + notnull->skip_validation = false; + notnull->initially_valid = true; + + cxt->nnconstraints = lappend(cxt->nnconstraints, notnull); + + /* Don't need this anymore, if we had it */ + need_notnull = false; + } + break; case CONSTR_DEFAULT: @@ -722,16 +768,19 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) column->identity = constraint->generated_when; saw_identity = true; - /* An identity column is implicitly NOT NULL */ - if (saw_nullable && !column->is_not_null) + /* + * Identity columns are always NOT NULL, but we may have a + * constraint already. + */ + if (!saw_nullable) + need_notnull = true; + else if (!column->is_not_null) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"", column->colname, cxt->relation->relname), parser_errposition(cxt->pstate, constraint->location))); - column->is_not_null = true; - saw_nullable = true; break; } @@ -755,6 +804,11 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) case CONSTR_CHECK: cxt->ckconstraints = lappend(cxt->ckconstraints, constraint); + + /* + * XXX If the user says CHECK (IS NOT NULL), should we turn + * that into a regular NOT NULL constraint? + */ break; case CONSTR_PRIMARY: @@ -837,6 +891,30 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) constraint->location))); } + /* + * If we need a NOT NULL constraint for SERIAL or IDENTITY, and one was + * not explicitly specified, add one now. + */ + if (need_notnull && !(saw_nullable && column->is_not_null)) + { + Constraint *notnull; + + column->is_not_null = true; + + notnull = makeNode(Constraint); + notnull->contype = CONSTR_NOTNULL; + notnull->conname = NULL; + notnull->deferrable = false; + notnull->initdeferred = false; + notnull->location = -1; + notnull->parent_oid = InvalidOid; + notnull->colname = column->colname; + notnull->skip_validation = false; + notnull->initially_valid = true; + + cxt->nnconstraints = lappend(cxt->nnconstraints, notnull); + } + /* * If needed, generate ALTER FOREIGN TABLE ALTER COLUMN statement to add * per-column foreign data wrapper options to this column after creation. @@ -912,6 +990,10 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint) cxt->ckconstraints = lappend(cxt->ckconstraints, constraint); break; + case CONSTR_NOTNULL: + cxt->nnconstraints = lappend(cxt->nnconstraints, constraint); + break; + case CONSTR_FOREIGN: if (cxt->isforeign) ereport(ERROR, @@ -923,7 +1005,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint) break; case CONSTR_NULL: - case CONSTR_NOTNULL: case CONSTR_DEFAULT: case CONSTR_ATTR_DEFERRABLE: case CONSTR_ATTR_NOT_DEFERRABLE: @@ -959,6 +1040,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla AclResult aclresult; char *comment; ParseCallbackState pcbstate; + bool process_notnull_constraints; setup_parser_errposition_callback(&pcbstate, cxt->pstate, table_like_clause->relation->location); @@ -1040,6 +1122,8 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla def->inhcount = 0; def->is_local = true; def->is_not_null = attribute->attnotnull; + if (attribute->attnotnull) + process_notnull_constraints = true; def->is_from_type = false; def->storage = 0; def->raw_default = NULL; @@ -1121,14 +1205,19 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla * we don't yet know what column numbers the copied columns will have in * the finished table. If any of those options are specified, add the * LIKE clause to cxt->likeclauses so that expandTableLikeClause will be - * called after we do know that. Also, remember the relation OID so that + * called after we do know that; in addition, do that if there are any NOT + * NULL constraints, because those must be propagated even if not + * explicitly requested. + * + * In order for this to work, we remember the relation OID so that * expandTableLikeClause is certain to open the same table. */ - if (table_like_clause->options & + if ((table_like_clause->options & (CREATE_TABLE_LIKE_DEFAULTS | CREATE_TABLE_LIKE_GENERATED | CREATE_TABLE_LIKE_CONSTRAINTS | - CREATE_TABLE_LIKE_INDEXES)) + CREATE_TABLE_LIKE_INDEXES)) || + process_notnull_constraints) { table_like_clause->relationOid = RelationGetRelid(relation); cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause); @@ -1200,6 +1289,7 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) TupleConstr *constr; AttrMap *attmap; char *comment; + ListCell *lc; /* * Open the relation referenced by the LIKE clause. We should still have @@ -1379,6 +1469,20 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) } } + /* + * Copy NOT NULL constraints, too (these do not require any option to have + * been given). + */ + foreach(lc, RelationGetNotNullConstraints(relation, false)) + { + AlterTableCmd *atsubcmd; + + atsubcmd = makeNode(AlterTableCmd); + atsubcmd->subtype = AT_AddConstraint; + atsubcmd->def = (Node *) lfirst_node(Constraint, lc); + atsubcmds = lappend(atsubcmds, atsubcmd); + } + /* * If we generated any ALTER TABLE actions above, wrap them into a single * ALTER TABLE command. Stick it at the front of the result, so it runs @@ -2065,10 +2169,12 @@ transformIndexConstraints(CreateStmtContext *cxt) ListCell *lc; /* - * Run through the constraints that need to generate an index. For PRIMARY - * KEY, mark each column as NOT NULL and create an index. For UNIQUE or - * EXCLUDE, create an index as for PRIMARY KEY, but do not insist on NOT - * NULL. + * Run through the constraints that need to generate an index, and do so. + * + * For PRIMARY KEY, in addition we set each column's attnotnull flag true. + * We do not create a separate CHECK (IS NOT NULL) constraint, as that + * would be redundant: the PRIMARY KEY constraint itself fulfills that + * role. Other constraint types don't need any NOT NULL markings. */ foreach(lc, cxt->ixconstraints) { @@ -2142,9 +2248,7 @@ transformIndexConstraints(CreateStmtContext *cxt) } /* - * Now append all the IndexStmts to cxt->alist. If we generated an ALTER - * TABLE SET NOT NULL statement to support a primary key, it's already in - * cxt->alist. + * Now append all the IndexStmts to cxt->alist. */ cxt->alist = list_concat(cxt->alist, finalindexlist); } @@ -2152,12 +2256,10 @@ transformIndexConstraints(CreateStmtContext *cxt) /* * transformIndexConstraint * Transform one UNIQUE, PRIMARY KEY, or EXCLUDE constraint for - * transformIndexConstraints. + * transformIndexConstraints. An IndexStmt is returned. * - * We return an IndexStmt. For a PRIMARY KEY constraint, we additionally - * produce NOT NULL constraints, either by marking ColumnDefs in cxt->columns - * as is_not_null or by adding an ALTER TABLE SET NOT NULL command to - * cxt->alist. + * For a PRIMARY KEY constraint, we additionally force the columns to be + * marked as NOT NULL, without producing a CHECK (IS NOT NULL) constraint. */ static IndexStmt * transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) @@ -2424,7 +2526,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) { char *key = strVal(lfirst(lc)); bool found = false; - bool forced_not_null = false; ColumnDef *column = NULL; ListCell *columns; IndexElem *iparam; @@ -2445,13 +2546,14 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) * column is defined in the new table. For PRIMARY KEY, we * can apply the NOT NULL constraint cheaply here ... unless * the column is marked is_from_type, in which case marking it - * here would be ineffective (see MergeAttributes). + * here would be ineffective (see MergeAttributes). Note that + * this isn't effective in ALTER TABLE either, unless the + * column is being added in the same command. */ if (constraint->contype == CONSTR_PRIMARY && !column->is_from_type) { column->is_not_null = true; - forced_not_null = true; } } else if (SystemAttributeByName(key) != NULL) @@ -2494,14 +2596,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) if (strcmp(key, inhname) == 0) { found = true; - - /* - * It's tempting to set forced_not_null if the - * parent column is already NOT NULL, but that - * seems unsafe because the column's NOT NULL - * marking might disappear between now and - * execution. Do the runtime check to be safe. - */ break; } } @@ -2555,15 +2649,11 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt) iparam->nulls_ordering = SORTBY_NULLS_DEFAULT; index->indexParams = lappend(index->indexParams, iparam); - /* - * For a primary-key column, also create an item for ALTER TABLE - * SET NOT NULL if we couldn't ensure it via is_not_null above. - */ - if (constraint->contype == CONSTR_PRIMARY && !forced_not_null) + if (constraint->contype == CONSTR_PRIMARY) { AlterTableCmd *notnullcmd = makeNode(AlterTableCmd); - notnullcmd->subtype = AT_SetNotNull; + notnullcmd->subtype = AT_SetAttNotNull; notnullcmd->name = pstrdup(key); notnullcmds = lappend(notnullcmds, notnullcmd); } @@ -3335,6 +3425,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, cxt.isalter = true; cxt.columns = NIL; cxt.ckconstraints = NIL; + cxt.nnconstraints = NIL; cxt.fkconstraints = NIL; cxt.ixconstraints = NIL; cxt.likeclauses = NIL; @@ -3578,8 +3669,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, /* * We assume here that cxt.alist contains only IndexStmts and possibly - * ALTER TABLE SET NOT NULL statements generated from primary key - * constraints. We absorb the subcommands of the latter directly. + * AT_SetAttNotNull statements generated from primary key constraints. + * We absorb the subcommands of the latter directly. */ if (IsA(istmt, IndexStmt)) { @@ -3607,14 +3698,21 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, { newcmd = makeNode(AlterTableCmd); newcmd->subtype = AT_AddConstraint; - newcmd->def = (Node *) lfirst(l); + newcmd->def = (Node *) lfirst_node(Constraint, l); newcmds = lappend(newcmds, newcmd); } foreach(l, cxt.fkconstraints) { newcmd = makeNode(AlterTableCmd); newcmd->subtype = AT_AddConstraint; - newcmd->def = (Node *) lfirst(l); + newcmd->def = (Node *) lfirst_node(Constraint, l); + newcmds = lappend(newcmds, newcmd); + } + foreach(l, cxt.nnconstraints) + { + newcmd = makeNode(AlterTableCmd); + newcmd->subtype = AT_AddConstraint; + newcmd->def = (Node *) lfirst_node(Constraint, l); newcmds = lappend(newcmds, newcmd); } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 6dc117dea8..79acfebde9 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2493,6 +2493,18 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, conForm->connoinherit ? " NO INHERIT" : ""); break; } + case CONSTRAINT_NOTNULL: + { + AttrNumber attnum; + + attnum = extractNotNullColumn(tup); + + appendStringInfo(&buf, "NOT NULL %s", + quote_identifier(get_attname(conForm->conrelid, + attnum, false))); + break; + } + case CONSTRAINT_TRIGGER: /* diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index d01ab504b6..19527399cb 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -37,7 +37,7 @@ typedef struct CookedConstraint ConstrType contype; /* CONSTR_DEFAULT or CONSTR_CHECK */ Oid conoid; /* constr OID if created, otherwise Invalid */ char *name; /* name, or NULL if none */ - AttrNumber attnum; /* which attr (only for DEFAULT) */ + AttrNumber attnum; /* which attr (only for NOTNULL, DEFAULT) */ Node *expr; /* transformed default or check expr */ bool skip_validation; /* skip validation? (only for CHECK) */ bool is_local; /* constraint has local (non-inherited) def */ @@ -113,6 +113,9 @@ extern List *AddRelationNewConstraints(Relation rel, bool is_local, bool is_internal, const char *queryString); +extern void AddRelationNotNullConstraints(Relation rel, + List *constraints, + List *additional_notnulls); extern void RelationClearMissing(Relation rel); extern void SetAttrMissing(Oid relid, char *attname, char *value); diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h index 96889fddfa..ace5d9351c 100644 --- a/src/include/catalog/pg_constraint.h +++ b/src/include/catalog/pg_constraint.h @@ -181,6 +181,7 @@ DECLARE_ARRAY_FOREIGN_KEY((confrelid, confkey), pg_attribute, (attrelid, attnum) /* Valid values for contype */ #define CONSTRAINT_CHECK 'c' #define CONSTRAINT_FOREIGN 'f' +#define CONSTRAINT_NOTNULL 'n' #define CONSTRAINT_PRIMARY 'p' #define CONSTRAINT_UNIQUE 'u' #define CONSTRAINT_TRIGGER 't' @@ -237,9 +238,6 @@ extern Oid CreateConstraintEntry(const char *constraintName, bool conNoInherit, bool is_internal); -extern void RemoveConstraintById(Oid conId); -extern void RenameConstraintById(Oid conId, const char *newname); - extern bool ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId, const char *conname); extern bool ConstraintNameExists(const char *conname, Oid namespaceid); @@ -247,6 +245,13 @@ extern char *ChooseConstraintName(const char *name1, const char *name2, const char *label, Oid namespaceid, List *others); +extern HeapTuple findNotNullConstraintAttnum(Relation rel, AttrNumber attnum); +extern HeapTuple findNotNullConstraint(Relation rel, const char *colname); +extern AttrNumber extractNotNullColumn(HeapTuple constrTup); + +extern void RemoveConstraintById(Oid conId); +extern void RenameConstraintById(Oid conId, const char *newname); + extern void AlterConstraintNamespaces(Oid ownerId, Oid oldNspId, Oid newNspId, bool isType, ObjectAddresses *objsMoved); extern void ConstraintSetParentConstraint(Oid childConstrId, diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index e7c2b91a58..3c6d65ada8 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -72,6 +72,8 @@ extern ObjectAddress renameatt(RenameStmt *stmt); extern ObjectAddress RenameConstraint(RenameStmt *stmt); +extern List *RelationGetNotNullConstraints(Relation relation, bool cooked); + extern ObjectAddress RenameRelation(RenameStmt *stmt); extern void RenameRelationInternal(Oid myrelid, diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index f7d7f10f7d..4031c98938 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2070,6 +2070,7 @@ typedef enum AlterTableType AT_CookedColumnDefault, /* add a pre-cooked column default */ AT_DropNotNull, /* alter column drop not null */ AT_SetNotNull, /* alter column set not null */ + AT_SetAttNotNull, /* set attnotnull w/o a constraint */ AT_DropExpression, /* alter column drop expression */ AT_CheckNotNull, /* check column is already marked not null */ AT_SetStatistics, /* alter column set statistics */ @@ -2355,10 +2356,11 @@ typedef struct VariableShowStmt * Create Table Statement * * NOTE: in the raw gram.y output, ColumnDef and Constraint nodes are - * intermixed in tableElts, and constraints is NIL. After parse analysis, - * tableElts contains just ColumnDefs, and constraints contains just - * Constraint nodes (in fact, only CONSTR_CHECK nodes, in the present - * implementation). + * intermixed in tableElts, and constraints and notnullcols are NIL. After + * parse analysis, tableElts contains just ColumnDefs, notnullcols has been + * filled with not-nullable column names from various sources, and constraints + * contains just Constraint nodes (in fact, only CONSTR_CHECK nodes, in the + * present implementation). * ---------------------- */ @@ -2373,6 +2375,7 @@ typedef struct CreateStmt PartitionSpec *partspec; /* PARTITION BY clause */ TypeName *ofTypename; /* OF typename */ List *constraints; /* constraints (list of Constraint nodes) */ + List *nnconstraints; /* NOT NULL constraints (ditto) */ List *options; /* options from WITH clause */ OnCommitAction oncommit; /* what do we do at COMMIT? */ char *tablespacename; /* table space to use, or NULL */ @@ -2454,6 +2457,7 @@ typedef struct Constraint bool deferrable; /* DEFERRABLE? */ bool initdeferred; /* INITIALLY DEFERRED? */ int location; /* token location, or -1 if unknown */ + Oid parent_oid; /* OID of parent constraint, if any */ /* Fields used for constraints with expressions (CHECK and DEFAULT): */ bool is_no_inherit; /* is constraint non-inheritable? */ @@ -2461,6 +2465,9 @@ typedef struct Constraint char *cooked_expr; /* expr, as nodeToString representation */ char generated_when; /* ALWAYS or BY DEFAULT */ + /* Fields used for "raw" NOT NULL constraints: */ + char *colname; /* column it applies to */ + /* Fields used for unique constraints (UNIQUE and PRIMARY KEY): */ bool nulls_not_distinct; /* null treatment for UNIQUE constraints */ List *keys; /* String nodes naming referenced key diff --git a/src/test/modules/test_ddl_deparse/expected/alter_table.out b/src/test/modules/test_ddl_deparse/expected/alter_table.out index 87a1ab7aab..4d8e3abfed 100644 --- a/src/test/modules/test_ddl_deparse/expected/alter_table.out +++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out @@ -28,6 +28,7 @@ ALTER TABLE parent ADD COLUMN b serial; NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type alter table, tag ALTER TABLE NOTICE: subcommand: type ADD COLUMN (and recurse) desc column b of table parent +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint parent_b_not_null on table parent NOTICE: DDL test: type simple, tag ALTER SEQUENCE ALTER TABLE parent RENAME COLUMN b TO c; NOTICE: DDL test: type simple, tag ALTER TABLE @@ -57,24 +58,18 @@ NOTICE: subcommand: type DETACH PARTITION desc table part2 DROP TABLE part2; ALTER TABLE part ADD PRIMARY KEY (a); NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type SET NOT NULL desc column a of table part -NOTICE: subcommand: type SET NOT NULL desc column a of table part1 +NOTICE: subcommand: type SET ATTNOTNULL desc +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: subcommand: type ADD INDEX desc index part_pkey ALTER TABLE parent ALTER COLUMN a SET NOT NULL; NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type SET NOT NULL desc column a of table parent -NOTICE: subcommand: type SET NOT NULL desc column a of table child -NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild +NOTICE: subcommand: type SET NOT NULL (and recurse) desc constraint parent_a_not_null on table parent ALTER TABLE parent ALTER COLUMN a DROP NOT NULL; NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type DROP NOT NULL desc column a of table parent -NOTICE: subcommand: type DROP NOT NULL desc column a of table child -NOTICE: subcommand: type DROP NOT NULL desc column a of table grandchild +NOTICE: subcommand: type DROP NOT NULL (and recurse) desc column a of table parent ALTER TABLE parent ALTER COLUMN a SET NOT NULL; NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type SET NOT NULL desc column a of table parent -NOTICE: subcommand: type SET NOT NULL desc column a of table child -NOTICE: subcommand: type SET NOT NULL desc column a of table grandchild +NOTICE: subcommand: type SET NOT NULL (and recurse) desc constraint parent_a_not_null on table parent ALTER TABLE parent ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type simple, tag ALTER SEQUENCE @@ -116,6 +111,7 @@ NOTICE: DDL test: type alter table, tag ALTER TABLE NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table parent NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table child NOTICE: subcommand: type ALTER COLUMN SET TYPE desc column c of table grandchild +NOTICE: subcommand: type (re) ADD CONSTRAINT desc constraint parent_b_not_null on table parent NOTICE: subcommand: type (re) ADD STATS desc statistics object parent_stat ALTER TABLE parent ALTER COLUMN c SET DEFAULT 0; NOTICE: DDL test: type alter table, tag ALTER TABLE diff --git a/src/test/modules/test_ddl_deparse/expected/create_table.out b/src/test/modules/test_ddl_deparse/expected/create_table.out index 2178ce83e9..dc9175bf77 100644 --- a/src/test/modules/test_ddl_deparse/expected/create_table.out +++ b/src/test/modules/test_ddl_deparse/expected/create_table.out @@ -54,6 +54,8 @@ NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type simple, tag CREATE SEQUENCE NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX NOTICE: DDL test: type simple, tag CREATE INDEX NOTICE: DDL test: type simple, tag ALTER SEQUENCE @@ -74,6 +76,8 @@ CREATE TABLE IF NOT EXISTS fkey_table ( EXCLUDE USING btree (check_col_2 WITH =) ); NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX NOTICE: DDL test: type simple, tag CREATE INDEX NOTICE: DDL test: type alter table, tag ALTER TABLE @@ -86,7 +90,7 @@ CREATE TABLE employees OF employee_type ( ); NOTICE: DDL test: type simple, tag CREATE TABLE NOTICE: DDL test: type alter table, tag ALTER TABLE -NOTICE: subcommand: type SET NOT NULL desc column name of table employees +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX -- Inheritance CREATE TABLE person ( @@ -96,6 +100,8 @@ CREATE TABLE person ( location point ); NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX CREATE TABLE emp ( salary int4, @@ -128,6 +134,10 @@ CREATE TABLE like_datatype_table ( EXCLUDING ALL ); NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint datatype_table_id_big_not_null on table like_datatype_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint datatype_table_id_not_null on table like_datatype_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint datatype_table_is_small_not_null on table like_datatype_table CREATE TABLE like_fkey_table ( LIKE fkey_table INCLUDING DEFAULTS @@ -137,6 +147,11 @@ CREATE TABLE like_fkey_table ( NOTICE: DDL test: type simple, tag CREATE TABLE NOTICE: DDL test: type alter table, tag ALTER TABLE NOTICE: subcommand: type ALTER COLUMN SET DEFAULT (precooked) desc column id of table like_fkey_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_big_id_not_null on table like_fkey_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_check_col_1_not_null on table like_fkey_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_check_col_2_not_null on table like_fkey_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_datatype_id_not_null on table like_fkey_table +NOTICE: subcommand: type ADD CONSTRAINT (and recurse) desc constraint fkey_table_id_not_null on table like_fkey_table NOTICE: DDL test: type simple, tag CREATE INDEX NOTICE: DDL test: type simple, tag CREATE INDEX -- Volatile table types @@ -144,21 +159,29 @@ CREATE UNLOGGED TABLE unlogged_table ( id INT PRIMARY KEY ); NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX CREATE TEMP TABLE temp_table ( id INT PRIMARY KEY ); NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX CREATE TEMP TABLE temp_table_commit_delete ( id INT PRIMARY KEY ) ON COMMIT DELETE ROWS; NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX CREATE TEMP TABLE temp_table_commit_drop ( id INT PRIMARY KEY ) ON COMMIT DROP; NOTICE: DDL test: type simple, tag CREATE TABLE +NOTICE: DDL test: type alter table, tag ALTER TABLE +NOTICE: subcommand: type SET ATTNOTNULL desc NOTICE: DDL test: type simple, tag CREATE INDEX diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c index b7c6f98577..da5079be47 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -129,6 +129,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS) case AT_SetNotNull: strtype = "SET NOT NULL"; break; + case AT_SetAttNotNull: + strtype = "SET ATTNOTNULL"; + break; case AT_DropExpression: strtype = "DROP EXPRESSION"; break; diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 97bfc3475b..d19349b301 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1119,9 +1119,13 @@ ERROR: relation "non_existent" does not exist create table atacc1 (test int not null); alter table atacc1 add constraint "atacc1_pkey" primary key (test); alter table atacc1 alter column test drop not null; -ERROR: column "test" is in a primary key alter table atacc1 drop constraint "atacc1_pkey"; -alter table atacc1 alter column test drop not null; +\d atacc1 + Table "public.atacc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + test | integer | | | + insert into atacc1 values (null); alter table atacc1 alter test set not null; ERROR: column "test" of relation "atacc1" contains null values @@ -1191,14 +1195,15 @@ alter table parent alter a drop not null; insert into parent values (NULL); insert into child (a, b) values (NULL, 'foo'); alter table only parent alter a set not null; -ERROR: column "a" of relation "parent" contains null values +ERROR: cannot add constraint to table with inheritance children +HINT: Do not specify the ONLY keyword. alter table child alter a set not null; ERROR: column "a" of relation "child" contains null values delete from parent; alter table only parent alter a set not null; +ERROR: cannot add constraint to table with inheritance children +HINT: Do not specify the ONLY keyword. insert into parent values (NULL); -ERROR: null value in column "a" of relation "parent" violates not-null constraint -DETAIL: Failing row contains (null). alter table child alter a set not null; insert into child (a, b) values (NULL, 'foo'); ERROR: null value in column "a" of relation "child" violates not-null constraint @@ -4318,8 +4323,7 @@ ERROR: cannot alter inherited column "b" -- cannot add/drop NOT NULL or check constraints to *only* the parent, when -- partitions exist ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL; -ERROR: constraint must be added to child tables too -DETAIL: Column "b" of relation "part_2" is not already NOT NULL. +ERROR: cannot add constraint to only the partitioned table when partitions exist HINT: Do not specify the ONLY keyword. ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz'); ERROR: constraint must be added to child tables too diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 2eec483eaa..14bc2f1cc3 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -247,11 +247,12 @@ ERROR: insert or update on table "clstr_tst" violates foreign key constraint "c DETAIL: Key (b)=(1111) is not present in table "clstr_tst_s". SELECT conname FROM pg_constraint WHERE conrelid = 'clstr_tst'::regclass ORDER BY 1; - conname ----------------- + conname +---------------------- + clstr_tst_a_not_null clstr_tst_con clstr_tst_pkey -(2 rows) +(3 rows) SELECT relname, relkind, EXISTS(SELECT 1 FROM pg_class WHERE oid = c.reltoastrelid) AS hastoast diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out index e6f6602d95..16c822504c 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -754,6 +754,97 @@ ALTER TABLE deferred_excl ADD EXCLUDE (f1 WITH =); ERROR: could not create exclusion constraint "deferred_excl_f1_excl" DETAIL: Key (f1)=(3) conflicts with key (f1)=(3). DROP TABLE deferred_excl; +-- verify CHECK constraints created for NOT NULL clauses +CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL); +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | + +-- DROP NOT NULL gets rid of both the attnotnull flag and the constraint itself +ALTER TABLE notnull_tbl1 ALTER a DROP NOT NULL; +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + +-- SET NOT NULL puts both back +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | + +-- The simple syntax must not create redundant constraint +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | + +-- but this should create a second one +ALTER TABLE notnull_tbl1 ADD check (a IS NOT NULL); +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | +Check constraints: + "notnull_tbl1_a_check" CHECK (a IS NOT NULL) + +-- Dropping the first one keeps attnotnull intact +ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_a_not_null; +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Check constraints: + "notnull_tbl1_a_check" CHECK (a IS NOT NULL) + +-- but removing the second constraint resets the flag +ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_a_not_null1; +ERROR: constraint "notnull_tbl1_a_not_null1" of relation "notnull_tbl1" does not exist +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Check constraints: + "notnull_tbl1_a_check" CHECK (a IS NOT NULL) + +DROP TABLE notnull_tbl1; +CREATE TABLE notnull_tbl2 (a INTEGER PRIMARY KEY); +ALTER TABLE notnull_tbl2 ALTER a DROP NOT NULL; +ERROR: column "a" is in a primary key +CREATE TABLE notnull_tbl3 (a INTEGER NOT NULL, CHECK (a IS NOT NULL)); +ALTER TABLE notnull_tbl3 ALTER A DROP NOT NULL; +ALTER TABLE notnull_tbl3 ADD b int, ADD CONSTRAINT pk PRIMARY KEY (a, b); +\d notnull_tbl3 + Table "public.notnull_tbl3" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | + b | integer | | not null | +Indexes: + "pk" PRIMARY KEY, btree (a, b) +Check constraints: + "notnull_tbl3_a_check" CHECK (a IS NOT NULL) + +ALTER TABLE notnull_tbl3 DROP CONSTRAINT pk; +\d notnull_tbl3 + Table "public.notnull_tbl3" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Check constraints: + "notnull_tbl3_a_check" CHECK (a IS NOT NULL) + -- Comments -- Setup a low-level role to enforce non-superuser checks. CREATE ROLE regress_constraint_comments; diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 5eace915a7..32102204a1 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -766,22 +766,24 @@ CREATE TABLE part_b PARTITION OF parted ( ) FOR VALUES IN ('b'); NOTICE: merging constraint "check_a" with inherited definition -- conislocal should be false for any merged constraints, true otherwise -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY conislocal, coninhcount; - conislocal | coninhcount -------------+------------- - f | 1 - t | 0 -(2 rows) +SELECT conname, conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount DESC, conname; + conname | conislocal | coninhcount +-------------------+------------+------------- + check_a | f | 1 + part_b_b_not_null | t | 1 + check_b | t | 0 +(3 rows) -- Once check_b is added to the parent, it should be made non-local for part_b ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0); NOTICE: merging constraint "check_b" with inherited definition -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; +SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount DESC, conname; conislocal | coninhcount ------------+------------- f | 1 f | 1 -(2 rows) + t | 1 +(3 rows) -- Neither check_a nor check_b are droppable from part_b ALTER TABLE part_b DROP CONSTRAINT check_a; @@ -792,10 +794,11 @@ ERROR: cannot drop inherited constraint "check_b" of relation "part_b" -- traditional inheritance where they will be left behind, because they would -- be local constraints. ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b; -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; - conislocal | coninhcount -------------+------------- -(0 rows) +SELECT conname, conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount; + conname | conislocal | coninhcount +-------------------+------------+------------- + part_b_b_not_null | t | 1 +(1 row) -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index b7937fb3bc..11276063bb 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -738,6 +738,14 @@ drop domain dnotnulltest cascade; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to column col2 of table domnotnull drop cascades to column col1 of table domnotnull +create domain dnotnulltest integer constraint dnn not null; +select conname, contype, contypid::regtype from pg_constraint c + where contypid = 'dnotnulltest'::regtype; + conname | contype | contypid +---------+---------+---------- +(0 rows) + +drop domain dnotnulltest; -- Test ALTER DOMAIN .. DEFAULT .. create table domdeftest (col1 ddef1); insert into domdeftest default values; diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 5a10958df5..2c8a6b2212 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -408,6 +408,7 @@ NOTICE: END: command_tag=CREATE SCHEMA type=schema identity=evttrig NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.one_col_a_seq NOTICE: END: command_tag=CREATE SEQUENCE type=sequence identity=evttrig.one_col_c_seq NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.one +NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.one NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.one_pkey NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_a_seq NOTICE: END: command_tag=ALTER SEQUENCE type=sequence identity=evttrig.one_col_c_seq @@ -422,6 +423,7 @@ CREATE TABLE evttrig.parted ( id int PRIMARY KEY) PARTITION BY RANGE (id); NOTICE: END: command_tag=CREATE TABLE type=table identity=evttrig.parted +NOTICE: END: command_tag=ALTER TABLE type=table identity=evttrig.parted NOTICE: END: command_tag=CREATE INDEX type=index identity=evttrig.parted_pkey CREATE TABLE evttrig.part_1_10 PARTITION OF evttrig.parted (id) FOR VALUES FROM (1) TO (10); diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 5b30ee49f3..e90f4f846b 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -1652,11 +1652,12 @@ SELECT relname, conname, contype, conislocal, coninhcount, connoinherit FROM pg_class AS pc JOIN pg_constraint AS pgc ON (conrelid = pc.oid) WHERE pc.relname = 'fd_pt1' ORDER BY 1,2; - relname | conname | contype | conislocal | coninhcount | connoinherit ----------+------------+---------+------------+-------------+-------------- - fd_pt1 | fd_pt1chk1 | c | t | 0 | t - fd_pt1 | fd_pt1chk2 | c | t | 0 | f -(2 rows) + relname | conname | contype | conislocal | coninhcount | connoinherit +---------+--------------------+---------+------------+-------------+-------------- + fd_pt1 | fd_pt1_c1_not_null | n | t | 0 | f + fd_pt1 | fd_pt1chk1 | c | t | 0 | t + fd_pt1 | fd_pt1chk2 | c | t | 0 | f +(3 rows) -- child does not inherit NO INHERIT constraints \d+ fd_pt1 diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 2f9c083539..c7b699d9df 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2035,13 +2035,19 @@ ORDER BY co.contype, cr.relname, co.conname, p.conname; part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk + part1_self_fk | part1_self_fk_id_not_null | n | t | | | + part2_self_fk | parted_self_fk_id_not_null | n | t | | | + part32_self_fk | part3_self_fk_id_not_null | n | t | | | + part33_self_fk | part33_self_fk_id_not_null | n | t | | | + part3_self_fk | part3_self_fk_id_not_null | n | t | | | + parted_self_fk | parted_self_fk_id_not_null | n | t | | | part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t | part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t | part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t | part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t | part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t | parted_self_fk | parted_self_fk_pkey | p | t | | | -(12 rows) +(18 rows) -- detach and re-attach multiple times just to ensure everything is kosher ALTER TABLE parted_self_fk DETACH PARTITION part2_self_fk; @@ -2064,13 +2070,19 @@ ORDER BY co.contype, cr.relname, co.conname, p.conname; part33_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk part3_self_fk | parted_self_fk_id_abc_fkey | f | t | parted_self_fk_id_abc_fkey | t | parted_self_fk parted_self_fk | parted_self_fk_id_abc_fkey | f | t | | | parted_self_fk + part1_self_fk | part1_self_fk_id_not_null | n | t | | | + part2_self_fk | parted_self_fk_id_not_null | n | t | | | + part32_self_fk | part3_self_fk_id_not_null | n | t | | | + part33_self_fk | part33_self_fk_id_not_null | n | t | | | + part3_self_fk | part3_self_fk_id_not_null | n | t | | | + parted_self_fk | parted_self_fk_id_not_null | n | t | | | part1_self_fk | part1_self_fk_pkey | p | t | parted_self_fk_pkey | t | part2_self_fk | part2_self_fk_pkey | p | t | parted_self_fk_pkey | t | part32_self_fk | part32_self_fk_pkey | p | t | part3_self_fk_pkey | t | part33_self_fk | part33_self_fk_pkey | p | t | part3_self_fk_pkey | t | part3_self_fk | part3_self_fk_pkey | p | t | parted_self_fk_pkey | t | parted_self_fk | parted_self_fk_pkey | p | t | | | -(12 rows) +(18 rows) -- Leave this table around, for pg_upgrade/pg_dump tests -- Test creating a constraint at the parent that already exists in partitions. diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 1bdd430f06..5351a87425 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -1065,16 +1065,18 @@ create table idxpart3 (b int not null, a int not null); alter table idxpart attach partition idxpart3 for values from (20, 20) to (30, 30); select conname, contype, conrelid::regclass, conindid::regclass, conkey from pg_constraint where conrelid::regclass::text like 'idxpart%' - order by conname; - conname | contype | conrelid | conindid | conkey -----------------+---------+-----------+----------------+-------- - idxpart1_pkey | p | idxpart1 | idxpart1_pkey | {1,2} - idxpart21_pkey | p | idxpart21 | idxpart21_pkey | {1,2} - idxpart22_pkey | p | idxpart22 | idxpart22_pkey | {1,2} - idxpart2_pkey | p | idxpart2 | idxpart2_pkey | {1,2} - idxpart3_pkey | p | idxpart3 | idxpart3_pkey | {2,1} - idxpart_pkey | p | idxpart | idxpart_pkey | {1,2} -(6 rows) + order by conrelid::regclass::text, conname; + conname | contype | conrelid | conindid | conkey +---------------------+---------+-----------+----------------+-------- + idxpart_pkey | p | idxpart | idxpart_pkey | {1,2} + idxpart1_pkey | p | idxpart1 | idxpart1_pkey | {1,2} + idxpart2_pkey | p | idxpart2 | idxpart2_pkey | {1,2} + idxpart21_pkey | p | idxpart21 | idxpart21_pkey | {1,2} + idxpart22_pkey | p | idxpart22 | idxpart22_pkey | {1,2} + idxpart3_a_not_null | n | idxpart3 | - | {2} + idxpart3_b_not_null | n | idxpart3 | - | {1} + idxpart3_pkey | p | idxpart3 | idxpart3_pkey | {2,1} +(8 rows) drop table idxpart; -- Verify that multi-layer partitioning honors the requirement that all @@ -1207,12 +1209,21 @@ create table idxpart (a int) partition by range (a); create table idxpart0 (like idxpart); alter table idxpart0 add unique (a); alter table idxpart attach partition idxpart0 default; -alter table only idxpart add primary key (a); -- fail, no NOT NULL constraint -ERROR: constraint must be added to child tables too -DETAIL: Column "a" of relation "idxpart0" is not already NOT NULL. -HINT: Do not specify the ONLY keyword. +alter table only idxpart add primary key (a); -- works, but idxpart0.a is nullable +\d idxpart0 + Table "public.idxpart0" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Partition of: idxpart DEFAULT +Indexes: + "idxpart0_a_key" UNIQUE CONSTRAINT, btree (a) + +alter index idxpart_pkey attach partition idxpart0_a_key; -- fails, lacks NOT NULL +ERROR: invalid primary key definition +DETAIL: Column "a" of relation "idxpart0" is not marked NOT NULL. alter table idxpart0 alter column a set not null; -alter table only idxpart add primary key (a); -- now it works +alter index idxpart_pkey attach partition idxpart0_a_key; alter table idxpart0 alter column a drop not null; -- fail, pkey needs it ERROR: column "a" is marked NOT NULL in parent table drop table idxpart; diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index e2a0dc80b2..4777499c21 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -860,35 +860,44 @@ DETAIL: Failing row contains (null). insert into bc (aa) values (NULL); ERROR: new row for relation "bc" violates check constraint "ac_aa_check" DETAIL: Failing row contains (null, null). -alter table bc drop constraint ac_aa_check; -- fail, disallowed -ERROR: cannot drop inherited constraint "ac_aa_check" of relation "bc" -alter table ac drop constraint ac_aa_check; +alter table bc drop constraint ac_aa_not_null; -- fail, disallowed +ERROR: constraint "ac_aa_not_null" of relation "bc" does not exist +alter table ac drop constraint ac_aa_not_null; +ERROR: constraint "ac_aa_not_null" of relation "ac" does not exist select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2; - relname | conname | contype | conislocal | coninhcount | consrc ----------+---------+---------+------------+-------------+-------- -(0 rows) + relname | conname | contype | conislocal | coninhcount | consrc +---------+-------------+---------+------------+-------------+------------------ + ac | ac_aa_check | c | t | 0 | (aa IS NOT NULL) + bc | ac_aa_check | c | f | 1 | (aa IS NOT NULL) +(2 rows) alter table ac add constraint ac_check check (aa is not null); alter table bc no inherit ac; select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2; - relname | conname | contype | conislocal | coninhcount | consrc ----------+----------+---------+------------+-------------+------------------ - ac | ac_check | c | t | 0 | (aa IS NOT NULL) - bc | ac_check | c | t | 0 | (aa IS NOT NULL) -(2 rows) + relname | conname | contype | conislocal | coninhcount | consrc +---------+-------------+---------+------------+-------------+------------------ + ac | ac_aa_check | c | t | 0 | (aa IS NOT NULL) + ac | ac_check | c | t | 0 | (aa IS NOT NULL) + bc | ac_aa_check | c | t | 0 | (aa IS NOT NULL) + bc | ac_check | c | t | 0 | (aa IS NOT NULL) +(4 rows) alter table bc drop constraint ac_check; select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2; - relname | conname | contype | conislocal | coninhcount | consrc ----------+----------+---------+------------+-------------+------------------ - ac | ac_check | c | t | 0 | (aa IS NOT NULL) -(1 row) + relname | conname | contype | conislocal | coninhcount | consrc +---------+-------------+---------+------------+-------------+------------------ + ac | ac_aa_check | c | t | 0 | (aa IS NOT NULL) + ac | ac_check | c | t | 0 | (aa IS NOT NULL) + bc | ac_aa_check | c | t | 0 | (aa IS NOT NULL) +(3 rows) alter table ac drop constraint ac_check; select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2; - relname | conname | contype | conislocal | coninhcount | consrc ----------+---------+---------+------------+-------------+-------- -(0 rows) + relname | conname | contype | conislocal | coninhcount | consrc +---------+-------------+---------+------------+-------------+------------------ + ac | ac_aa_check | c | t | 0 | (aa IS NOT NULL) + bc | ac_aa_check | c | t | 0 | (aa IS NOT NULL) +(2 rows) drop table bc; drop table ac; @@ -1847,6 +1856,343 @@ select * from cnullparent where f1 = 2; drop table cnullparent cascade; NOTICE: drop cascades to table cnullchild -- +-- Test inheritance of NOT NULL constraints +-- +create table pp1 (f1 int); +create table cc1 (f2 text, f3 int) inherits (pp1); +\d cc1 + Table "public.cc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | + f2 | text | | | + f3 | integer | | | +Inherits: pp1 + +create table cc2(f4 float) inherits(pp1,cc1); +NOTICE: merging multiple inherited definitions of column "f1" +\d cc2 + Table "public.cc2" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+--------- + f1 | integer | | | + f2 | text | | | + f3 | integer | | | + f4 | double precision | | | +Inherits: pp1, + cc1 + +-- named NOT NULL constraint +alter table cc1 add column a2 int constraint nn not null; +\d cc1 + Table "public.cc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | + f2 | text | | | + f3 | integer | | | + a2 | integer | | not null | +Inherits: pp1 +Number of child tables: 1 (Use \d+ to list them.) + +\d cc2 + Table "public.cc2" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+--------- + f1 | integer | | | + f2 | text | | | + f3 | integer | | | + f4 | double precision | | | + a2 | integer | | not null | +Inherits: pp1, + cc1 + +alter table pp1 alter column f1 set not null; +\d pp1 + Table "public.pp1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Number of child tables: 2 (Use \d+ to list them.) + +\d cc1 + Table "public.cc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | + f2 | text | | | + f3 | integer | | | + a2 | integer | | not null | +Inherits: pp1 +Number of child tables: 1 (Use \d+ to list them.) + +\d cc2 + Table "public.cc2" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+--------- + f1 | integer | | not null | + f2 | text | | | + f3 | integer | | | + f4 | double precision | | | + a2 | integer | | not null | +Inherits: pp1, + cc1 + +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+-----------------+---------+-------------+------------ + cc1 | nn | n | 0 | t + cc2 | nn | n | 1 | f + pp1 | pp1_f1_not_null | n | 0 | t + cc1 | pp1_f1_not_null | n | 1 | f + cc2 | pp1_f1_not_null | n | 1 | f +(5 rows) + +-- remove constraint from cc2; one is gone, the other stays +alter table cc2 alter column a2 drop not null; +ERROR: cannot drop inherited constraint "nn" of relation "cc2" +-- remove constraint cc1, should succeed +alter table cc1 alter column a2 drop not null; +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+-----------------+---------+-------------+------------ + pp1 | pp1_f1_not_null | n | 0 | t + cc1 | pp1_f1_not_null | n | 1 | f + cc2 | pp1_f1_not_null | n | 1 | f +(3 rows) + +-- same for cc2 +alter table cc2 alter column f1 drop not null; +ERROR: cannot drop inherited constraint "pp1_f1_not_null" of relation "cc2" +-- remove from cc1, should fail again +alter table cc1 alter column f1 drop not null; +ERROR: cannot drop inherited constraint "pp1_f1_not_null" of relation "cc1" +-- remove from pp1, should succeed +alter table pp1 alter column f1 drop not null; +ERROR: constraint "pp1_f1_not_null" of relation "cc2" does not exist +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+-----------------+---------+-------------+------------ + pp1 | pp1_f1_not_null | n | 0 | t + cc1 | pp1_f1_not_null | n | 1 | f + cc2 | pp1_f1_not_null | n | 1 | f +(3 rows) + +drop table pp1 cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table cc1 +drop cascades to table cc2 +\d cc1 +\d cc2 +-- +-- test inherit/deinherit +-- +create table inh_parent(f1 int); +create table inh_child1(f1 int not null); +create table inh_child2(f1 int); +-- inh_child1 should have not null constraint +alter table inh_child1 inherit inh_parent; +-- should fail, missing NOT NULL constraint +alter table inh_child2 inherit inh_child1; +ERROR: column "f1" in child table must be marked NOT NULL +alter table inh_child2 alter column f1 set not null; +alter table inh_child2 inherit inh_child1; +-- add NOT NULL constraint recursively +alter table inh_parent alter column f1 set not null; +\d inh_parent + Table "public.inh_parent" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Number of child tables: 1 (Use \d+ to list them.) + +\d inh_child1 + Table "public.inh_child1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Inherits: inh_parent +Number of child tables: 1 (Use \d+ to list them.) + +\d inh_child2 + Table "public.inh_child2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Inherits: inh_child1 + +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child1'::regclass, 'inh_child2'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +------------+------------------------+---------+-------------+------------ + inh_child1 | inh_child1_f1_not_null | n | 0 | t + inh_child2 | inh_child2_f1_not_null | n | 0 | t + inh_parent | inh_parent_f1_not_null | n | 0 | t +(3 rows) + +-- +-- test deinherit procedure +-- +-- deinherit inh_child1 +alter table inh_child1 no inherit inh_parent; +\d inh_parent + Table "public.inh_parent" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | + +\d inh_child1 + Table "public.inh_child1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Number of child tables: 1 (Use \d+ to list them.) + +\d inh_child2 + Table "public.inh_child2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | not null | +Inherits: inh_child1 + +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child1'::regclass, 'inh_child2'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +------------+------------------------+---------+-------------+------------ + inh_child1 | inh_child1_f1_not_null | n | 0 | t + inh_child2 | inh_child2_f1_not_null | n | 0 | t + inh_parent | inh_parent_f1_not_null | n | 0 | t +(3 rows) + +-- test inhcount of inh_child2, should fail +alter table inh_child2 alter f1 drop not null; +-- should succeed +drop table inh_parent; +drop table inh_child1 cascade; +NOTICE: drop cascades to table inh_child2 +-- +-- test multi inheritance tree +-- +create table inh_parent(f1 int not null); +create table c1() inherits(inh_parent); +create table c2() inherits(inh_parent); +create table d1() inherits(c1, c2); +NOTICE: merging multiple inherited definitions of column "f1" +-- show constraint info +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'c1'::regclass, 'c2'::regclass, 'd1'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +------------+------------------------+---------+-------------+------------ + inh_parent | inh_parent_f1_not_null | n | 0 | t + c1 | inh_parent_f1_not_null | n | 1 | f + c2 | inh_parent_f1_not_null | n | 1 | f + d1 | inh_parent_f1_not_null | n | 1 | f +(4 rows) + +drop table inh_parent cascade; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table c1 +drop cascades to table c2 +drop cascades to table d1 +-- test child table with inherited columns and +-- with explicitely specified not null constraints +create table inh_parent_1(f1 int); +create table inh_parent_2(f2 text); +create table child(f1 int not null, f2 text not null) inherits(inh_parent_1, inh_parent_2); +NOTICE: merging column "f1" with inherited definition +NOTICE: merging column "f2" with inherited definition +-- show constraint info +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent_1'::regclass, 'inh_parent_2'::regclass, 'child'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+-------------------+---------+-------------+------------ + child | child_f1_not_null | n | 0 | t + child | child_f2_not_null | n | 0 | t +(2 rows) + +-- also drops child table +drop table inh_parent_1 cascade; +NOTICE: drop cascades to table child +drop table inh_parent_2; +-- test multi layer inheritance tree +create table inh_p1(f1 int not null); +create table inh_p2(f1 int not null); +create table inh_p3(f2 int); +create table inh_p4(f1 int not null, f3 text not null); +create table c() inherits(inh_p1, inh_p2, inh_p3, inh_p4); +NOTICE: merging multiple inherited definitions of column "f1" +NOTICE: merging multiple inherited definitions of column "f1" +ERROR: relation "c" already exists +-- constraint on f1 should have three parents +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_p1'::regclass, 'inh_p2'::regclass, 'inh_p3'::regclass, 'inh_p4'::regclass, 'c'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+--------------------+---------+-------------+------------ + inh_p1 | inh_p1_f1_not_null | n | 0 | t + inh_p2 | inh_p2_f1_not_null | n | 0 | t + inh_p4 | inh_p4_f1_not_null | n | 0 | t + inh_p4 | inh_p4_f3_not_null | n | 0 | t +(4 rows) + +create table d(a int not null, f1 int) inherits(inh_p3, c); +ERROR: relation "d" already exists +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_p1'::regclass, 'inh_p2'::regclass, 'inh_p3'::regclass, 'inh_p4'::regclass, 'c'::regclass, 'd'::regclass) + order by 2, 1; + conrelid | conname | contype | coninhcount | conislocal +----------+--------------------+---------+-------------+------------ + inh_p1 | inh_p1_f1_not_null | n | 0 | t + inh_p2 | inh_p2_f1_not_null | n | 0 | t + inh_p4 | inh_p4_f1_not_null | n | 0 | t + inh_p4 | inh_p4_f3_not_null | n | 0 | t +(4 rows) + +drop table inh_p1 cascade; +drop table inh_p2; +drop table inh_p3; +drop table inh_p4; +-- Verify constraint renaming when recursing to child +create schema inh1 create table onetab (a int); +create schema inh2 create table onetab (b int) inherits (inh1.onetab); +alter table inh2.onetab add constraint onetab_a_not_null check (b > 0); +alter table inh2.onetab add constraint foobar not null a; +-- fails: target constraint name in use, when renaming existing constraint +alter table inh1.onetab alter a set not null; +ERROR: cannot rename constraint on relation "inh2.onetab" to "onetab_a_not_null" +DETAIL: Another constraint with that name already exists. +alter table inh2.onetab drop constraint foobar; +-- fails: target constraint name in use, when creating new constraint +alter table inh1.onetab alter a set not null; +ERROR: cannot add constraint "onetab_a_not_null" to relation "inh2.onetab" +DETAIL: Another constraint with that name already exists. +drop schema inh1, inh2 cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table inh1.onetab +drop cascades to table inh2.onetab +-- -- Check use of temporary tables with inheritance trees -- create table inh_perm_parent (a1 int); diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out index 7d798ef2a5..9571840d25 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -263,8 +263,21 @@ Indexes: "test_replica_identity4_pkey" PRIMARY KEY, btree (id) REPLICA IDENTITY Partitions: test_replica_identity4_1 FOR VALUES IN (1) +-- Dropping the primary key is not allowed if that would leave the replica +-- identity as nullable +CREATE TABLE test_replica_identity5 (a int not null, b int, c int, + PRIMARY KEY (b, c)); +CREATE UNIQUE INDEX test_replica_identity5_a_b_key ON test_replica_identity5 (a, b); +ALTER TABLE test_replica_identity5 REPLICA IDENTITY USING INDEX test_replica_identity5_a_b_key; +ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; +ERROR: column "b" is in index used as replica identity +ALTER TABLE test_replica_identity5 ALTER b SET NOT NULL; +ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; +ALTER TABLE test_replica_identity5 ALTER b DROP NOT NULL; +ERROR: column "b" is in index used as replica identity DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; DROP TABLE test_replica_identity4; +DROP TABLE test_replica_identity5; DROP TABLE test_replica_identity_othertable; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index b5d57a771a..99b09a5328 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -852,7 +852,7 @@ create table atacc1 (test int not null); alter table atacc1 add constraint "atacc1_pkey" primary key (test); alter table atacc1 alter column test drop not null; alter table atacc1 drop constraint "atacc1_pkey"; -alter table atacc1 alter column test drop not null; +\d atacc1 insert into atacc1 values (null); alter table atacc1 alter test set not null; delete from atacc1; diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 5ffcd4ffc7..ae427d25e9 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -556,6 +556,39 @@ ALTER TABLE deferred_excl ADD EXCLUDE (f1 WITH =); DROP TABLE deferred_excl; +-- verify CHECK constraints created for NOT NULL clauses +CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL); +\d notnull_tbl1 +-- DROP NOT NULL gets rid of both the attnotnull flag and the constraint itself +ALTER TABLE notnull_tbl1 ALTER a DROP NOT NULL; +\d notnull_tbl1 +-- SET NOT NULL puts both back +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +\d notnull_tbl1 +-- The simple syntax must not create redundant constraint +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +\d notnull_tbl1 +-- but this should create a second one +ALTER TABLE notnull_tbl1 ADD check (a IS NOT NULL); +\d notnull_tbl1 +-- Dropping the first one keeps attnotnull intact +ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_a_not_null; +\d notnull_tbl1 +-- but removing the second constraint resets the flag +ALTER TABLE notnull_tbl1 DROP CONSTRAINT notnull_tbl1_a_not_null1; +\d notnull_tbl1 +DROP TABLE notnull_tbl1; + +CREATE TABLE notnull_tbl2 (a INTEGER PRIMARY KEY); +ALTER TABLE notnull_tbl2 ALTER a DROP NOT NULL; + +CREATE TABLE notnull_tbl3 (a INTEGER NOT NULL, CHECK (a IS NOT NULL)); +ALTER TABLE notnull_tbl3 ALTER A DROP NOT NULL; +ALTER TABLE notnull_tbl3 ADD b int, ADD CONSTRAINT pk PRIMARY KEY (a, b); +\d notnull_tbl3 +ALTER TABLE notnull_tbl3 DROP CONSTRAINT pk; +\d notnull_tbl3 + -- Comments -- Setup a low-level role to enforce non-superuser checks. CREATE ROLE regress_constraint_comments; diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 93ccf77d4a..18f92b73da 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -532,11 +532,11 @@ CREATE TABLE part_b PARTITION OF parted ( CONSTRAINT check_b CHECK (b >= 0) ) FOR VALUES IN ('b'); -- conislocal should be false for any merged constraints, true otherwise -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY conislocal, coninhcount; +SELECT conname, conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount DESC, conname; -- Once check_b is added to the parent, it should be made non-local for part_b ALTER TABLE parted ADD CONSTRAINT check_b CHECK (b >= 0); -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; +SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount DESC, conname; -- Neither check_a nor check_b are droppable from part_b ALTER TABLE part_b DROP CONSTRAINT check_a; @@ -546,7 +546,7 @@ ALTER TABLE part_b DROP CONSTRAINT check_b; -- traditional inheritance where they will be left behind, because they would -- be local constraints. ALTER TABLE parted DROP CONSTRAINT check_a, DROP CONSTRAINT check_b; -SELECT conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass; +SELECT conname, conislocal, coninhcount FROM pg_constraint WHERE conrelid = 'part_b'::regclass ORDER BY coninhcount; -- specify PARTITION BY for a partition CREATE TABLE fail_part_col_not_found PARTITION OF parted FOR VALUES IN ('c') PARTITION BY RANGE (c); diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index a9a56f5277..75703940f9 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -427,6 +427,13 @@ update domnotnull set col1 = null; drop domain dnotnulltest cascade; +create domain dnotnulltest integer constraint dnn not null; + +select conname, contype, contypid::regtype from pg_constraint c + where contypid = 'dnotnulltest'::regtype; + +drop domain dnotnulltest; + -- Test ALTER DOMAIN .. DEFAULT .. create table domdeftest (col1 ddef1); diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 429120e710..e60f3fb932 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -522,7 +522,7 @@ create table idxpart3 (b int not null, a int not null); alter table idxpart attach partition idxpart3 for values from (20, 20) to (30, 30); select conname, contype, conrelid::regclass, conindid::regclass, conkey from pg_constraint where conrelid::regclass::text like 'idxpart%' - order by conname; + order by conrelid::regclass::text, conname; drop table idxpart; -- Verify that multi-layer partitioning honors the requirement that all @@ -620,9 +620,11 @@ create table idxpart (a int) partition by range (a); create table idxpart0 (like idxpart); alter table idxpart0 add unique (a); alter table idxpart attach partition idxpart0 default; -alter table only idxpart add primary key (a); -- fail, no NOT NULL constraint +alter table only idxpart add primary key (a); -- works, but idxpart0.a is nullable +\d idxpart0 +alter index idxpart_pkey attach partition idxpart0_a_key; -- fails, lacks NOT NULL alter table idxpart0 alter column a set not null; -alter table only idxpart add primary key (a); -- now it works +alter index idxpart_pkey attach partition idxpart0_a_key; alter table idxpart0 alter column a drop not null; -- fail, pkey needs it drop table idxpart; diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 5db6dbc191..0b75f6ce1b 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -279,8 +279,8 @@ select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg insert into ac (aa) values (NULL); insert into bc (aa) values (NULL); -alter table bc drop constraint ac_aa_check; -- fail, disallowed -alter table ac drop constraint ac_aa_check; +alter table bc drop constraint ac_aa_not_null; -- fail, disallowed +alter table ac drop constraint ac_aa_not_null; select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2; alter table ac add constraint ac_check check (aa is not null); @@ -679,6 +679,184 @@ select * from cnullparent; select * from cnullparent where f1 = 2; drop table cnullparent cascade; +-- +-- Test inheritance of NOT NULL constraints +-- +create table pp1 (f1 int); +create table cc1 (f2 text, f3 int) inherits (pp1); +\d cc1 +create table cc2(f4 float) inherits(pp1,cc1); +\d cc2 + +-- named NOT NULL constraint +alter table cc1 add column a2 int constraint nn not null; +\d cc1 +\d cc2 +alter table pp1 alter column f1 set not null; +\d pp1 +\d cc1 +\d cc2 + +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + +-- remove constraint from cc2; one is gone, the other stays +alter table cc2 alter column a2 drop not null; + +-- remove constraint cc1, should succeed +alter table cc1 alter column a2 drop not null; + +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + +-- same for cc2 +alter table cc2 alter column f1 drop not null; + +-- remove from cc1, should fail again +alter table cc1 alter column f1 drop not null; + +-- remove from pp1, should succeed +alter table pp1 alter column f1 drop not null; + +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + +drop table pp1 cascade; +\d cc1 +\d cc2 + +-- +-- test inherit/deinherit +-- +create table inh_parent(f1 int); +create table inh_child1(f1 int not null); +create table inh_child2(f1 int); + +-- inh_child1 should have not null constraint +alter table inh_child1 inherit inh_parent; + +-- should fail, missing NOT NULL constraint +alter table inh_child2 inherit inh_child1; + +alter table inh_child2 alter column f1 set not null; +alter table inh_child2 inherit inh_child1; + +-- add NOT NULL constraint recursively +alter table inh_parent alter column f1 set not null; + +\d inh_parent +\d inh_child1 +\d inh_child2 + +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child1'::regclass, 'inh_child2'::regclass) + order by 2, 1; + +-- +-- test deinherit procedure +-- + +-- deinherit inh_child1 +alter table inh_child1 no inherit inh_parent; +\d inh_parent +\d inh_child1 +\d inh_child2 +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child1'::regclass, 'inh_child2'::regclass) + order by 2, 1; + +-- test inhcount of inh_child2, should fail +alter table inh_child2 alter f1 drop not null; + +-- should succeed + +drop table inh_parent; +drop table inh_child1 cascade; + +-- +-- test multi inheritance tree +-- +create table inh_parent(f1 int not null); +create table c1() inherits(inh_parent); +create table c2() inherits(inh_parent); +create table d1() inherits(c1, c2); + +-- show constraint info +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'c1'::regclass, 'c2'::regclass, 'd1'::regclass) + order by 2, 1; + +drop table inh_parent cascade; + +-- test child table with inherited columns and +-- with explicitely specified not null constraints +create table inh_parent_1(f1 int); +create table inh_parent_2(f2 text); +create table child(f1 int not null, f2 text not null) inherits(inh_parent_1, inh_parent_2); + +-- show constraint info +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent_1'::regclass, 'inh_parent_2'::regclass, 'child'::regclass) + order by 2, 1; + +-- also drops child table +drop table inh_parent_1 cascade; +drop table inh_parent_2; + +-- test multi layer inheritance tree +create table inh_p1(f1 int not null); +create table inh_p2(f1 int not null); +create table inh_p3(f2 int); +create table inh_p4(f1 int not null, f3 text not null); + +create table c() inherits(inh_p1, inh_p2, inh_p3, inh_p4); + +-- constraint on f1 should have three parents +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_p1'::regclass, 'inh_p2'::regclass, 'inh_p3'::regclass, 'inh_p4'::regclass, 'c'::regclass) + order by 2, 1; + +create table d(a int not null, f1 int) inherits(inh_p3, c); + +select conrelid::regclass, conname, contype, coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('inh_p1'::regclass, 'inh_p2'::regclass, 'inh_p3'::regclass, 'inh_p4'::regclass, 'c'::regclass, 'd'::regclass) + order by 2, 1; + +drop table inh_p1 cascade; +drop table inh_p2; +drop table inh_p3; +drop table inh_p4; + +-- Verify constraint renaming when recursing to child +create schema inh1 create table onetab (a int); +create schema inh2 create table onetab (b int) inherits (inh1.onetab); +alter table inh2.onetab add constraint onetab_a_not_null check (b > 0); +alter table inh2.onetab add constraint foobar not null a; +-- fails: target constraint name in use, when renaming existing constraint +alter table inh1.onetab alter a set not null; + +alter table inh2.onetab drop constraint foobar; +-- fails: target constraint name in use, when creating new constraint +alter table inh1.onetab alter a set not null; + +drop schema inh1, inh2 cascade; + + -- -- Check use of temporary tables with inheritance trees -- diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql index 14620b7713..5748b34162 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -117,8 +117,20 @@ ALTER INDEX test_replica_identity4_pkey ATTACH PARTITION test_replica_identity4_1_pkey; \d+ test_replica_identity4 +-- Dropping the primary key is not allowed if that would leave the replica +-- identity as nullable +CREATE TABLE test_replica_identity5 (a int not null, b int, c int, + PRIMARY KEY (b, c)); +CREATE UNIQUE INDEX test_replica_identity5_a_b_key ON test_replica_identity5 (a, b); +ALTER TABLE test_replica_identity5 REPLICA IDENTITY USING INDEX test_replica_identity5_a_b_key; +ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; +ALTER TABLE test_replica_identity5 ALTER b SET NOT NULL; +ALTER TABLE test_replica_identity5 DROP CONSTRAINT test_replica_identity5_pkey; +ALTER TABLE test_replica_identity5 ALTER b DROP NOT NULL; + DROP TABLE test_replica_identity; DROP TABLE test_replica_identity2; DROP TABLE test_replica_identity3; DROP TABLE test_replica_identity4; +DROP TABLE test_replica_identity5; DROP TABLE test_replica_identity_othertable; -- 2.30.2