From 6b84c2d220d924b51b9bcc436473d404ec0950ca Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 15 Mar 2023 20:11:47 +0100 Subject: [PATCH v7] Catalog NOT NULL constraints Each declared NOT NULL constraint now gets a corresponding pg_constraint row. --- doc/src/sgml/catalogs.sgml | 1 + doc/src/sgml/ref/alter_table.sgml | 8 +- doc/src/sgml/ref/create_table.sgml | 8 +- src/backend/catalog/heap.c | 491 ++++-- src/backend/catalog/pg_constraint.c | 98 ++ src/backend/commands/tablecmds.c | 1353 +++++++++++++---- src/backend/nodes/outfuncs.c | 4 + src/backend/nodes/readfuncs.c | 8 +- src/backend/optimizer/util/plancat.c | 2 + src/backend/parser/gram.y | 13 + src/backend/parser/parse_utilcmd.c | 210 ++- src/backend/utils/adt/ruleutils.c | 14 + src/bin/pg_dump/common.c | 15 +- src/bin/pg_dump/pg_backup_archiver.c | 2 + src/bin/pg_dump/pg_dump.c | 198 ++- src/bin/pg_dump/pg_dump.h | 2 +- src/bin/pg_dump/t/002_pg_dump.pl | 6 +- src/include/catalog/heap.h | 5 +- src/include/catalog/pg_constraint.h | 11 +- src/include/commands/tablecmds.h | 2 + src/include/nodes/parsenodes.h | 14 +- .../test_ddl_deparse/expected/alter_table.out | 18 +- .../expected/create_table.out | 25 +- .../test_ddl_deparse/test_ddl_deparse.c | 4 + src/test/regress/expected/alter_table.out | 50 +- src/test/regress/expected/cluster.out | 7 +- src/test/regress/expected/constraints.out | 114 ++ src/test/regress/expected/create_table.out | 27 +- 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 | 397 +++++ .../regress/expected/replica_identity.out | 13 + src/test/regress/parallel_schedule | 3 +- src/test/regress/sql/alter_table.sql | 26 +- src/test/regress/sql/constraints.sql | 43 + src/test/regress/sql/create_table.sql | 6 +- src/test/regress/sql/indexing.sql | 8 +- src/test/regress/sql/inherit.sql | 207 +++ src/test/regress/sql/replica_identity.sql | 12 + 41 files changed, 2862 insertions(+), 633 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 7c09ab3000..296f38c8a9 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -2552,6 +2552,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/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index d4d93eeb7c..b3b531486e 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -117,7 +117,9 @@ WITH ( MODULUS numeric_literal, REM PRIMARY KEY ( column_name [, ... ] ) index_parameters | EXCLUDE [ USING index_method ] ( exclude_element WITH operator [, ... ] ) index_parameters [ WHERE ( predicate ) ] | FOREIGN KEY ( column_name [, ... ] ) REFERENCES reftable [ ( refcolumn [, ... ] ) ] - [ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE referential_action ] [ ON UPDATE referential_action ] } + [ MATCH FULL | MATCH PARTIAL | MATCH SIMPLE ] [ ON DELETE referential_action ] [ ON UPDATE referential_action ] | + NOT NULL column_name +} [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ] and table_constraint_using_index is: @@ -1763,7 +1765,9 @@ ALTER TABLE measurement Compatibility - The forms ADD (without USING INDEX), + The forms ADD (without USING INDEX, and + except for the NOT NULL column_name + form to add a table constraint), DROP [COLUMN], DROP IDENTITY, RESTART, SET DEFAULT, SET DATA TYPE (without USING), SET GENERATED, and SET sequence_option diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 10ef699fab..22fdd8bac2 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 ) ] | @@ -2314,13 +2315,6 @@ CREATE TABLE cities_partdef constraint, and index names must be unique across all relations within the same schema. - - - Currently, PostgreSQL does not record names - for NOT NULL constraints at all, so they are not - subject to the uniqueness restriction. This might change in a future - release. - diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 2a0d82aedd..b3830a01b4 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); + } } /* @@ -2637,6 +2746,190 @@ 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 constraints 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. + * + * Returns a list of AttrNumber for columns that need to have the attnotnull + * column set. + */ +List * +AddRelationNotNullConstraints(Relation rel, List *constraints, + List *old_notnulls) +{ + List *nnnames = NIL; + List *givennames = NIL; + List *nncols = 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, constr->is_no_inherit); + + nncols = lappend_int(nncols, attnum); + } + + /* + * 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, + cooked->is_no_inherit); + + nncols = lappend_int(nncols, cooked->attnum); + + prev_attnum = cooked->attnum; + } + + return nncols; +} + /* * 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 4002317f70..d044dc46c6 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -562,6 +562,104 @@ 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; + + /* only tuples for CHECK constraints should be given */ + Assert(conForm->contype == CONSTRAINT_NOTNULL); + + adatum = SysCacheGetAttrNotNull(CONSTROID, constrTup, + Anum_pg_constraint_conkey); + 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 d9bbeafd82..30711d7be0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -203,7 +203,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 */ @@ -350,7 +351,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); @@ -431,14 +433,16 @@ 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 void set_attnotnull(List **wqueue, Relation rel, + AttrNumber attnum, bool recurse, LOCKMODE lockmode); +static ObjectAddress ATExecSetNotNull(List **wqueue, Relation rel, + char *constrname, char *colName, + bool recurse, bool recursing, + List **readyRels, LOCKMODE lockmode); +static void ATExecSetAttNotNull(List **wqueue, 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); @@ -541,6 +545,11 @@ 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, List **readyRels, + LOCKMODE lockmode); static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, @@ -616,7 +625,7 @@ static void RemoveInheritance(Relation child_rel, Relation parent_rel, static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, AlterTableUtilityContext *context); -static void AttachPartitionEnsureIndexes(Relation rel, Relation attachrel); +static void AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel); static void QueuePartitionConstraintValidation(List **wqueue, Relation scanrel, List *partConstraint, bool validate_default); @@ -634,6 +643,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); @@ -671,8 +681,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, TupleDesc descriptor; List *inheritOids; List *old_constraints; + List *old_notnulls; List *rawDefaults; List *cookedDefaults; + List *nncols; Datum reloptions; ListCell *listptr; AttrNumber attnum; @@ -862,12 +874,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); @@ -1250,6 +1263,17 @@ 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), create them, and set the attnotnull flag on + * columns that don't yet have it. + */ + nncols = AddRelationNotNullConstraints(rel, stmt->nnconstraints, + old_notnulls); + foreach(listptr, nncols) + set_attnotnull(NULL, rel, lfirst_int(listptr), false, NoLock); + ObjectAddressSet(address, RelationRelationId, relationId); /* @@ -2298,6 +2322,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. @@ -2328,7 +2354,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. @@ -2347,10 +2376,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 */ @@ -2462,9 +2492,13 @@ 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; + Bitmapset *nncols = NULL; + /* caller already got lock */ relation = table_open(parent, NoLock); @@ -2553,6 +2587,20 @@ 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 be + * NOT NULL; if partition just the attnotnull bit, otherwise a full + * constraint (if they don't have one already). Also, we request + * attnotnull on columns that have a NOT NULL constraint that's not + * marked NO INHERIT. + */ + pkattrs = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + nnconstrs = RelationGetNotNullConstraints(relation, true); + foreach(lc1, nnconstrs) + nncols = bms_add_member(nncols, + ((CookedConstraint *) lfirst(lc1))->attnum); + for (parent_attno = 1; parent_attno <= tupleDesc->natts; parent_attno++) { @@ -2648,9 +2696,38 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } /* - * Merge of NOT NULL constraints = OR 'em together + * 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. */ - def->is_not_null |= attribute->attnotnull; + 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); + } + + /* + * mark attnotnull if parent has it and it's not NO INHERIT + */ + if (bms_is_member(parent_attno, nncols) || + bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber, + pkattrs)) + def->is_not_null = true; /* * Check for GENERATED conflicts @@ -2684,7 +2761,11 @@ MergeAttributes(List *schema, List *supers, char relpersistence, attribute->atttypmod); def->inhcount = 1; def->is_local = false; - def->is_not_null = attribute->attnotnull; + /* mark attnotnull if parent has it and it's not NO INHERIT */ + if (bms_is_member(parent_attno, nncols) || + bms_is_member(parent_attno - FirstLowInvalidHeapAttributeNumber, + pkattrs)) + def->is_not_null = true; def->is_from_type = false; def->storage = attribute->attstorage; def->raw_default = NULL; @@ -2701,6 +2782,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); + } } /* @@ -2845,6 +2953,19 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } } + /* + * Also copy the NOT NULL constraints from this parent. The + * attnotnull markings were already installed above. + */ + foreach(lc1, nnconstrs) + { + CookedConstraint *nn = lfirst(lc1); + + nn->attnum = newattmap->attnums[nn->attnum - 1]; + + nnconstraints = lappend(nnconstraints, nn); + } + free_attrmap(newattmap); /* @@ -3051,8 +3172,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) { @@ -3158,6 +3278,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } *supconstr = constraints; + *supnotnulls = nnconstraints; + return schema; } @@ -3209,6 +3331,85 @@ 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. + * + * XXX This is only used to create derived tables, so NO INHERIT constraints + * are always skipped. + */ +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; + if (conForm->connoinherit) + 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 @@ -3769,7 +3970,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) { @@ -4354,6 +4558,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: @@ -4652,15 +4857,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 */ @@ -5045,10 +5258,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, rel, NULL, cmd->name, + cmd->recurse, false, NULL, lockmode); + break; + case AT_SetAttNotNull: /* set pg_attribute.attnotnull */ + ATExecSetAttNotNull(wqueue, rel, cmd->name, lockmode); break; case AT_CheckNotNull: /* check column is already marked NOT NULL */ ATExecCheckNotNull(tab, rel, cmd->name, lockmode); @@ -5387,11 +5604,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: @@ -5759,6 +5973,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; @@ -5819,6 +6034,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); @@ -5841,7 +6062,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 @@ -6067,6 +6288,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; @@ -6175,6 +6397,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: @@ -6774,8 +6998,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) { @@ -7290,41 +7513,19 @@ 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_attribute attTup; AttrNumber attnum; Relation attr_rel; - List *indexoidlist; - ListCell *indexoidscan; ObjectAddress address; /* @@ -7340,6 +7541,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) @@ -7355,68 +7565,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) @@ -7428,22 +7613,33 @@ 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); + dropconstraint_internal(rel, conTup, DROP_RESTRICT, recurse, false, + false, NULL, lockmode); + + heap_freetuple(conTup); table_close(attr_rel, RowExclusiveLock); @@ -7451,102 +7647,126 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) } /* - * ALTER TABLE ALTER COLUMN SET NOT NULL + * Helper to set pg_attribute.attnotnull if it isn't set, and to tell phase 3 + * to verify it; recurses to apply the same to children. + * + * When called to alter an existing table, 'wqueue' must be given so that we can + * queue a check that existing tuples pass the constraint. When called from + * table creation, 'wqueue' should be passed as NULL. */ - static void -ATPrepSetNotNull(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode, AlterTableUtilityContext *context) +set_attnotnull(List **wqueue, Relation rel, AttrNumber attnum, bool recurse, + LOCKMODE lockmode) { - /* - * If we're already recursing, there's nothing to do; the topmost - * invocation of ATSimpleRecursion already visited all children. - */ - if (recursing) + HeapTuple tuple; + Form_pg_attribute attForm; + List *children; + ListCell *lc; + + tuple = SearchSysCacheCopyAttNum(RelationGetRelid(rel), attnum); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for attribute %d of relation %u", + attnum, RelationGetRelid(rel)); + attForm = (Form_pg_attribute) GETSTRUCT(tuple); + if (!attForm->attnotnull) + { + Relation attr_rel; + + attr_rel = table_open(AttributeRelationId, RowExclusiveLock); + + attForm->attnotnull = true; + CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple); + + table_close(attr_rel, RowExclusiveLock); + + /* + * And set up for existing values to be checked, unless another constraint + * already proves this. + */ + if (wqueue && !NotNullImpliedByRelConstraints(rel, attForm)) + { + AlteredTableInfo *tab; + + tab = ATGetQueueEntry(wqueue, rel); + tab->verify_new_notnull = true; + } + } + + /* if no recursion is desired, we're done */ + if (!recurse) 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) + children = find_inheritance_children(RelationGetRelid(rel), lockmode); + foreach(lc, children) { - HeapTuple tuple; - bool attnotnull; + Oid childrelid = lfirst_oid(lc); + Relation childrel; + AttrNumber childattno; - tuple = SearchSysCacheAttName(RelationGetRelid(rel), cmd->name); + /* find_inheritance_children already got lock */ + childrel = table_open(childrelid, NoLock); + CheckTableNotInUse(childrel, "ALTER TABLE"); - /* 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; + childattno = get_attnum(RelationGetRelid(childrel), + get_attname(RelationGetRelid(rel), attnum, + false)); + set_attnotnull(wqueue, childrel, childattno, + recurse, lockmode); + table_close(childrel, NoLock); } - - /* - * 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. + * ALTER TABLE ALTER COLUMN SET NOT NULL + * + * 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. */ static ObjectAddress -ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, - const char *colName, LOCKMODE lockmode) +ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName, + bool recurse, bool recursing, List **readyRels, + LOCKMODE lockmode) { HeapTuple tuple; + Relation constr_rel; + ScanKeyData skey; + SysScanDesc conscan; AttrNumber attnum; - Relation attr_rel; ObjectAddress address; + Constraint *constraint; + CookedConstraint *ccon; + 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, @@ -7554,42 +7774,167 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, errmsg("cannot alter system column \"%s\"", colName))); - /* - * Okay, actually perform the catalog change ... if needed - */ - if (!((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull) - { - ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull = true; + /* 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); - CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple); + 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); + conForm = (Form_pg_constraint) GETSTRUCT(copytup); /* - * 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 we find an appropriate constraint, we're almost done, but just + * need to change some properties on it: if we're recursing, increment + * coninhcount; if not, set conislocal if not already set. */ - if (!tab->verify_new_notnull && - !NotNullImpliedByRelConstraints(rel, (Form_pg_attribute) GETSTRUCT(tuple))) + if (recursing) { - /* Tell Phase 3 it needs to test the constraint */ - tab->verify_new_notnull = true; + conForm->coninhcount++; + changed = true; + } + else if (!conForm->conislocal) + { + conForm->conislocal = true; + changed = true; } - ObjectAddressSubSet(address, RelationRelationId, - RelationGetRelid(rel), attnum); + if (changed) + { + CatalogTupleUpdate(constr_rel, ©tup->t_self, copytup); + ObjectAddressSet(address, ConstraintRelationId, conForm->oid); + } + + systable_endscan(conscan); + table_close(constr_rel, RowExclusiveLock); + + if (changed) + return address; + else + return InvalidObjectAddress; } - else - address = InvalidObjectAddress; - InvokeObjectPostAlterHook(RelationRelationId, - RelationGetRelid(rel), attnum); + systable_endscan(conscan); + table_close(constr_rel, RowExclusiveLock); - table_close(attr_rel, RowExclusiveLock); + /* + * 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 only to table with inheritance children"), + errhint("Do not specify the ONLY keyword.")); + } + + /* + * No constraint exists; we must add one. First determine a name to use, + * if we haven't already. + */ + 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->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); + + /* + * Mark pg_attribute.attnotnull for the column. Tell that function not to + * recurse, because we're going to do it here. + */ + set_attnotnull(wqueue, rel, attnum, false, lockmode); + + /* + * Recurse to propagate the constraint to children that don't have one. + */ + if (recurse) + { + List *children; + ListCell *lc; + + children = find_inheritance_children(RelationGetRelid(rel), + lockmode); + + foreach(lc, children) + { + Relation childrel; + + childrel = table_open(lfirst_oid(lc), NoLock); + + ATExecSetNotNull(wqueue, 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(List **wqueue, Relation rel, + const char *colName, LOCKMODE lockmode) +{ + AttrNumber attnum; + + attnum = get_attnum(RelationGetRelid(rel), colName); + if (attnum == InvalidAttrNumber) /* XXX should not happen .. elog? */ + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + colName, RelationGetRelationName(rel))); + + set_attnotnull(wqueue, rel, attnum, false, lockmode); +} + /* * ALTER TABLE ALTER COLUMN CHECK NOT NULL * @@ -8872,13 +9217,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, @@ -8963,9 +9309,9 @@ ChooseForeignKeyConstraintNameAddition(List *colnames) } /* - * Add a check 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. + * Add a check or 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. * * Subroutine for ATExecAddConstraint. * @@ -9023,6 +9369,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; @@ -9034,6 +9381,14 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, if (constr->conname == NULL) constr->conname = ccon->name; + /* + * If adding a NOT NULL constraint, set the pg_attribute flag and + * tell phase 3 to verify existing rows, if needed. + */ + if (constr->contype == CONSTR_NOTNULL) + set_attnotnull(wqueue, rel, ccon->attnum, + !ccon->is_no_inherit, lockmode); + ObjectAddressSet(address, ConstraintRelationId, ccon->conoid); } @@ -11958,16 +12313,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) @@ -11996,47 +12346,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, NULL, lockmode); found = true; } @@ -12045,31 +12356,248 @@ 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, List **readyRels, + 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; + char *colname; /* to match NOT NULL constraints when recursing */ + List *ready = NIL; + + if (readyRels == NULL) + readyRels = &ready; + if (list_member_oid(*readyRels, RelationGetRelid(rel))) + return InvalidObjectAddress; + *readyRels = lappend_oid(*readyRels, RelationGetRelid(rel)); + + conrel = table_open(ConstraintRelationId, RowExclusiveLock); + + con = (Form_pg_constraint) GETSTRUCT(constraintTup); + constrName = NameStr(con->conname); + + /* + * If the constraint is marked conislocal and is also inherited, then we + * just set conislocal false and we're done. The constraint doesn't go + * away, and we don't modify any children. + */ + if (con->conislocal && con->coninhcount > 0) + { + HeapTuple copytup; + + /* make a copy we can scribble on */ + copytup = heap_copytuple(constraintTup); + con = (Form_pg_constraint) GETSTRUCT(copytup); + con->conislocal = false; + CatalogTupleUpdate(conrel, ©tup->t_self, copytup); + + table_close(conrel, RowExclusiveLock); + + ObjectAddressSet(conobj, ConstraintRelationId, con->oid); + return conobj; + } + + /* 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 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 + * exists, 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; } /* @@ -12094,50 +12622,104 @@ ATExecDropConstraint(Relation rel, const char *constrName, errmsg("cannot remove constraint from only the partitioned table when partitions exist"), errhint("Do not specify the ONLY keyword."))); + /* For NOT NULL constraints we recurse by column name */ + if (con->contype == CONSTRAINT_NOTNULL) + colname = NameStr(TupleDescAttr(RelationGetDescr(rel), + linitial_int(unconstrained_cols) - 1)->attname); + else + colname = NULL; /* keep compiler quiet */ + foreach(child, children) { Oid childrelid = lfirst_oid(child); Relation childrel; + HeapTuple tuple; + Form_pg_constraint childcon; HeapTuple copy_tuple; + SysScanDesc scan; + ScanKeyData skey[3]; + + if (list_member_oid(*readyRels, childrelid)) + continue; /* child already processed */ /* find_inheritance_children already got lock */ childrel = table_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); - ScanKeyInit(&skey[0], - Anum_pg_constraint_conrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(childrelid)); - ScanKeyInit(&skey[1], - Anum_pg_constraint_contypid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(InvalidOid)); - ScanKeyInit(&skey[2], - Anum_pg_constraint_conname, - BTEqualStrategyNumber, F_NAMEEQ, - CStringGetDatum(constrName)); - scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, - true, NULL, 3, skey); + /* + * We search for NOT NULL constraint by column number, and other + * constraints by name. + */ + if (con->contype == CONSTRAINT_NOTNULL) + { + bool found = false; + AttrNumber child_colnum; + HeapTuple child_tup; - /* There can be at most one matching row */ - if (!HeapTupleIsValid(tuple = systable_getnext(scan))) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("constraint \"%s\" of relation \"%s\" does not exist", - constrName, - RelationGetRelationName(childrel)))); + child_colnum = get_attnum(RelationGetRelid(childrel), colname); + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(childrelid)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 1, skey); + while (HeapTupleIsValid(child_tup = systable_getnext(scan))) + { + Form_pg_constraint constr = (Form_pg_constraint) GETSTRUCT(child_tup); + AttrNumber constr_colnum; - copy_tuple = heap_copytuple(tuple); + if (constr->contype != CONSTRAINT_NOTNULL) + continue; + constr_colnum = extractNotNullColumn(child_tup); + if (constr_colnum != child_colnum) + continue; - systable_endscan(scan); + found = true; + break; /* found it */ + } + if (!found) /* shouldn't happen? */ + elog(ERROR, "failed to find NOT NULL constraint for column \"%s\" in table \"%s\"", + colname, RelationGetRelationName(childrel)); - con = (Form_pg_constraint) GETSTRUCT(copy_tuple); + copy_tuple = heap_copytuple(child_tup); + systable_endscan(scan); + } + else + { + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(childrelid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); + /* There can only be one, so no need to loop */ + tuple = systable_getnext(scan); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("constraint \"%s\" of relation \"%s\" does not exist", + constrName, + RelationGetRelationName(childrel)))); + copy_tuple = heap_copytuple(tuple); + systable_endscan(scan); + } - /* Right now only CHECK constraints can be inherited */ - if (con->contype != CONSTRAINT_CHECK) - elog(ERROR, "inherited constraint is not a CHECK constraint"); + childcon = (Form_pg_constraint) GETSTRUCT(copy_tuple); - if (con->coninhcount <= 0) /* shouldn't happen */ + /* Right now only CHECK and NOT NULL constraints can be inherited */ + if (childcon->contype != CONSTRAINT_CHECK && + childcon->contype != CONSTRAINT_NOTNULL) + elog(ERROR, "inherited constraint is not a CHECK or NOT NULL constraint"); + + if (childcon->coninhcount <= 0) /* shouldn't happen */ elog(ERROR, "relation %u has non-inherited constraint \"%s\"", childrelid, constrName); @@ -12147,17 +12729,17 @@ ATExecDropConstraint(Relation rel, const char *constrName, * If the child constraint has other definition sources, just * decrement its inheritance count; if not, recurse to delete it. */ - if (con->coninhcount == 1 && !con->conislocal) + if (childcon->coninhcount == 1 && !childcon->conislocal) { /* Time to delete this child constraint, too */ - ATExecDropConstraint(childrel, constrName, behavior, - true, true, - false, lockmode); + dropconstraint_internal(childrel, copy_tuple, behavior, + recurse, true, missing_ok, readyRels, + lockmode); } else { /* Child constraint must survive my deletion */ - con->coninhcount--; + childcon->coninhcount--; CatalogTupleUpdate(conrel, ©_tuple->t_self, copy_tuple); /* Make update visible */ @@ -12171,8 +12753,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, * need to mark the inheritors' constraints as locally defined * rather than inherited. */ - con->coninhcount--; - con->conislocal = true; + childcon->coninhcount--; + childcon->conislocal = true; CatalogTupleUpdate(conrel, ©_tuple->t_self, copy_tuple); @@ -12186,6 +12768,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, } table_close(conrel, RowExclusiveLock); + + return conobj; } /* @@ -13511,10 +14095,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 @@ -15258,6 +15842,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); @@ -15271,7 +15856,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); @@ -15283,7 +15868,8 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) HeapTuple child_tuple; bool found = false; - if (parent_con->contype != CONSTRAINT_CHECK) + if (parent_con->contype != CONSTRAINT_CHECK && + parent_con->contype != CONSTRAINT_NOTNULL) continue; /* if the parent's constraint is marked NO INHERIT, it's not inherited */ @@ -15303,14 +15889,30 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) Form_pg_constraint child_con = (Form_pg_constraint) GETSTRUCT(child_tuple); HeapTuple child_copy; - if (child_con->contype != CONSTRAINT_CHECK) + if (child_con->contype != parent_con->contype) continue; - if (strcmp(NameStr(parent_con->conname), + /* + * CHECK constraint are matched by name, NOT NULL ones by + * attribute number + */ + if (child_con->contype == CONSTRAINT_CHECK && + strcmp(NameStr(parent_con->conname), NameStr(child_con->conname)) != 0) continue; + else if (child_con->contype == CONSTRAINT_NOTNULL) + { + AttrNumber parent_attno = extractNotNullColumn(parent_tuple); + AttrNumber child_attno = extractNotNullColumn(child_tuple); - if (!constraints_equivalent(parent_tuple, child_tuple, tuple_desc)) + if (strcmp(get_attname(parent_relid, parent_attno, false), + get_attname(RelationGetRelid(child_rel), child_attno, + false)) != 0) + continue; + } + + if (child_con->contype == CONSTRAINT_CHECK && + !constraints_equivalent(parent_tuple, child_tuple, tuple_desc)) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("child table \"%s\" has different definition for check constraint \"%s\"", @@ -15518,6 +16120,7 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) HeapTuple attributeTuple, constraintTuple; List *connames; + List *nncolumns; bool found; bool child_is_partition = false; @@ -15588,6 +16191,8 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) * this, we first need a list of the names of the parent's check * constraints. (We cheat a bit by only checking for name matches, * assuming that the expressions will match.) + * + * For NOT NULL columns, we store column numbers to match. */ catalogRelation = table_open(ConstraintRelationId, RowExclusiveLock); ScanKeyInit(&key[0], @@ -15598,6 +16203,7 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) true, NULL, 1, key); connames = NIL; + nncolumns = NIL; while (HeapTupleIsValid(constraintTuple = systable_getnext(scan))) { @@ -15605,6 +16211,8 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) if (con->contype == CONSTRAINT_CHECK) connames = lappend(connames, pstrdup(NameStr(con->conname))); + if (con->contype == CONSTRAINT_NOTNULL) + nncolumns = lappend_int(nncolumns, extractNotNullColumn(constraintTuple)); } systable_endscan(scan); @@ -15620,21 +16228,40 @@ RemoveInheritance(Relation child_rel, Relation parent_rel, bool expect_detached) while (HeapTupleIsValid(constraintTuple = systable_getnext(scan))) { Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(constraintTuple); - bool match; + bool match = false; ListCell *lc; - if (con->contype != CONSTRAINT_CHECK) - continue; - - match = false; - foreach(lc, connames) + /* + * Match CHECK constraints by name, NOT NULL constraints by column + * number, and ignore all others. + */ + if (con->contype == CONSTRAINT_CHECK) { - if (strcmp(NameStr(con->conname), (char *) lfirst(lc)) == 0) + foreach(lc, connames) { - match = true; - break; + if (con->contype == CONSTRAINT_CHECK && + strcmp(NameStr(con->conname), (char *) lfirst(lc)) == 0) + { + match = true; + break; + } } } + else if (con->contype == CONSTRAINT_NOTNULL) + { + AttrNumber child_attno = extractNotNullColumn(constraintTuple); + + foreach(lc, nncolumns) + { + if (lfirst_int(lc) == child_attno) + { + match = true; + break; + } + } + } + else + continue; if (match) { @@ -17925,7 +18552,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, StorePartitionBound(attachrel, rel, cmd->bound); /* Ensure there exists a correct set of indexes in the partition. */ - AttachPartitionEnsureIndexes(rel, attachrel); + AttachPartitionEnsureIndexes(wqueue, rel, attachrel); /* and triggers */ CloneRowTriggersToPartition(rel, attachrel); @@ -18038,13 +18665,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, * partitioned table. */ static void -AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) +AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel) { List *idxes; List *attachRelIdxs; Relation *attachrelIdxRels; IndexInfo **attachInfos; - int i; ListCell *cell; MemoryContext cxt; MemoryContext oldcxt; @@ -18060,14 +18686,13 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) attachInfos = palloc(sizeof(IndexInfo *) * list_length(attachRelIdxs)); /* Build arrays of all existing indexes and their IndexInfos */ - i = 0; foreach(cell, attachRelIdxs) { Oid cldIdxId = lfirst_oid(cell); + int i = foreach_current_index(cell); attachrelIdxRels[i] = index_open(cldIdxId, AccessShareLock); attachInfos[i] = BuildIndexInfo(attachrelIdxRels[i]); - i++; } /* @@ -18133,7 +18758,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) * the first matching, unattached one we find, if any, as partition of * the parent index. If we find one, we're done. */ - for (i = 0; i < list_length(attachRelIdxs); i++) + for (int i = 0; i < list_length(attachRelIdxs); i++) { Oid cldIdxId = RelationGetRelid(attachrelIdxRels[i]); Oid cldConstrOid = InvalidOid; @@ -18189,6 +18814,28 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) stmt = generateClonedIndexStmt(NULL, idxRel, attmap, &conOid); + + /* + * If the index is a primary key, mark all columns as NOT NULL if + * they aren't already. + */ + if (stmt->primary) + { + MemoryContextSwitchTo(oldcxt); + for (int j = 0; j < info->ii_NumIndexKeyAttrs; j++) + { + AttrNumber childattno; + + childattno = get_attnum(RelationGetRelid(attachrel), + get_attname(RelationGetRelid(rel), + info->ii_IndexAttrNumbers[j], + false)); + set_attnotnull(wqueue, attachrel, childattno, + true, AccessExclusiveLock); + } + MemoryContextSwitchTo(cxt); + } + DefineIndex(RelationGetRelid(attachrel), stmt, InvalidOid, RelationGetRelid(idxRel), conOid, @@ -18201,7 +18848,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) out: /* Clean up. */ - for (i = 0; i < list_length(attachRelIdxs); i++) + for (int i = 0; i < list_length(attachRelIdxs); i++) index_close(attachrelIdxRels[i], AccessShareLock); MemoryContextSwitchTo(oldcxt); MemoryContextDelete(cxt); @@ -19102,6 +19749,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)) @@ -19238,6 +19892,29 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) } } +/* + * When attaching an index as a partition of a partitioned index which is a + * primary key, verify that all the columns in the partition are marked NOT + * NULL. + */ +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..d6d67c9083 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -717,6 +717,10 @@ _outConstraint(StringInfo str, const Constraint *node) case CONSTR_NOTNULL: appendStringInfoString(str, "NOT_NULL"); + WRITE_BOOL_FIELD(is_no_inherit); + 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 597e5b3ea8..4c200485f3 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -390,10 +390,16 @@ _readConstraint(void) switch (local_node->contype) { case CONSTR_NULL: - case CONSTR_NOTNULL: /* no extra fields */ break; + case CONSTR_NOTNULL: + READ_BOOL_FIELD(is_no_inherit); + 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 e3824efe9b..75d8445914 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 acf6cf4866..86692bf395 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4099,6 +4099,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, + &n->is_no_inherit, 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 b0f6fe4fa6..5a420aef1c 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); @@ -533,10 +536,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); @@ -634,10 +638,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... */ @@ -655,7 +657,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\"", @@ -667,15 +669,58 @@ 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->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: @@ -725,16 +770,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; } @@ -758,6 +806,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: @@ -840,6 +893,29 @@ 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->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. @@ -915,6 +991,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, @@ -926,7 +1006,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: @@ -962,6 +1041,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); @@ -1043,6 +1123,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; @@ -1124,14 +1206,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); @@ -1203,6 +1290,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 @@ -1382,6 +1470,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 @@ -2059,10 +2161,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) { @@ -2136,9 +2240,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); } @@ -2146,12 +2248,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) @@ -2417,7 +2517,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; @@ -2438,13 +2537,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) @@ -2487,14 +2587,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; } } @@ -2548,15 +2640,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); } @@ -3328,6 +3416,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; @@ -3571,8 +3660,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)) { @@ -3600,14 +3689,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 461735e84f..0242cf24b4 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2472,6 +2472,20 @@ 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))); + if (((Form_pg_constraint) GETSTRUCT(tup))->connoinherit) + appendStringInfoString(&buf, " NO INHERIT"); + break; + } + case CONSTRAINT_TRIGGER: /* diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 5d988986ed..ed95dc8dc6 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -82,7 +82,8 @@ static catalogid_hash *catalogIdHash = NULL; static void flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables, InhInfo *inhinfo, int numInherits); static void flagInhIndexes(Archive *fout, TableInfo *tblinfo, int numTables); -static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables); +static void flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, + int numTables); static int strInArray(const char *pattern, char **arr, int arr_size); static IndxInfo *findIndexByOid(Oid oid); @@ -226,7 +227,7 @@ getSchemaData(Archive *fout, int *numTablesPtr) getTableAttrs(fout, tblinfo, numTables); pg_log_info("flagging inherited columns in subtables"); - flagInhAttrs(fout->dopt, tblinfo, numTables); + flagInhAttrs(fout, fout->dopt, tblinfo, numTables); pg_log_info("reading partitioning data"); getPartitioningInfo(fout); @@ -471,7 +472,8 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * What we need to do here is: * * - Detect child columns that inherit NOT NULL bits from their parents, so - * that we needn't specify that again for the child. + * that we needn't specify that again for the child. (Versions >= 16 no + * longer need this.) * * - Detect child columns that have DEFAULT NULL when their parents had some * non-null default. In this case, we make up a dummy AttrDefInfo object so @@ -491,7 +493,7 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * modifies tblinfo */ static void -flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables) +flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables) { int i, j, @@ -572,8 +574,9 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables) } } - /* Remember if we found inherited NOT NULL */ - tbinfo->inhNotNull[j] = foundNotNull; + /* In versions < 16, remember if we found inherited NOT NULL */ + if (fout->remoteVersion < 160000) + tbinfo->localNotNull[j] = !foundNotNull; /* * Manufacture a DEFAULT NULL clause if necessary. This breaks diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index d518349e10..5b858b2348 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -601,6 +601,7 @@ RestoreArchive(Archive *AHX) if (strcmp(te->desc, "CONSTRAINT") == 0 || strcmp(te->desc, "CHECK CONSTRAINT") == 0 || + strcmp(te->desc, "NOT NULL CONSTRAINT") == 0 || strcmp(te->desc, "FK CONSTRAINT") == 0) strcpy(buffer, "DROP CONSTRAINT"); else @@ -3511,6 +3512,7 @@ _getObjectDescription(PQExpBuffer buf, const TocEntry *te) /* these object types don't have separate owners */ else if (strcmp(type, "CAST") == 0 || strcmp(type, "CHECK CONSTRAINT") == 0 || + strcmp(type, "NOT NULL CONSTRAINT") == 0 || strcmp(type, "CONSTRAINT") == 0 || strcmp(type, "DATABASE PROPERTIES") == 0 || strcmp(type, "DEFAULT") == 0 || diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 7a504dfe25..229c4ab316 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -8372,6 +8372,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) PQExpBuffer q = createPQExpBuffer(); PQExpBuffer tbloids = createPQExpBuffer(); PQExpBuffer checkoids = createPQExpBuffer(); + PQExpBuffer defaultoids = createPQExpBuffer(); PGresult *res; int ntups; int curtblindx; @@ -8389,6 +8390,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) int i_attalign; int i_attislocal; int i_attnotnull; + int i_localnotnull; int i_attoptions; int i_attcollation; int i_attcompression; @@ -8398,16 +8400,17 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) /* * We want to perform just one query against pg_attribute, and then just - * one against pg_attrdef (for DEFAULTs) and one against pg_constraint - * (for CHECK constraints). However, we mustn't try to select every row - * of those catalogs and then sort it out on the client side, because some - * of the server-side functions we need would be unsafe to apply to tables - * we don't have lock on. Hence, we build an array of the OIDs of tables - * we care about (and now have lock on!), and use a WHERE clause to - * constrain which rows are selected. + * one against pg_attrdef (for DEFAULTs) and two against pg_constraint + * (for CHECK constraints and for NOT NULL constraints). However, we + * mustn't try to select every row of those catalogs and then sort it out + * on the client side, because some of the server-side functions we need + * would be unsafe to apply to tables we don't have lock on. Hence, we + * build an array of the OIDs of tables we care about (and now have lock + * on!), and use a WHERE clause to constrain which rows are selected. */ appendPQExpBufferChar(tbloids, '{'); appendPQExpBufferChar(checkoids, '{'); + appendPQExpBufferChar(defaultoids, '{'); for (int i = 0; i < numTables; i++) { TableInfo *tbinfo = &tblinfo[i]; @@ -8451,7 +8454,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "a.attstattarget,\n" "a.attstorage,\n" "t.typstorage,\n" - "a.attnotnull,\n" "a.atthasdef,\n" "a.attisdropped,\n" "a.attlen,\n" @@ -8468,6 +8470,17 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "ORDER BY option_name" "), E',\n ') AS attfdwoptions,\n"); + /* + * Write out NOT NULL, except if it's marked connoinherit. + */ + if (fout->remoteVersion >= 160000) + appendPQExpBufferStr(q, + "co.conname IS NOT NULL AS attnotnull,\n" + "co.conislocal AS local_notnull,\n"); + else + appendPQExpBufferStr(q, + "a.attnotnull, false AS local_notnull,\n"); + if (fout->remoteVersion >= 140000) appendPQExpBufferStr(q, "a.attcompression AS attcompression,\n"); @@ -8502,10 +8515,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_attribute a ON (src.tbloid = a.attrelid) " "LEFT JOIN pg_catalog.pg_type t " - "ON (a.atttypid = t.oid)\n" - "WHERE a.attnum > 0::pg_catalog.int2\n" - "ORDER BY a.attrelid, a.attnum", + "ON (a.atttypid = t.oid)\n", tbloids->data); + if (fout->remoteVersion >= 160000) + appendPQExpBufferStr(q, + " LEFT JOIN pg_catalog.pg_constraint co ON (a.attrelid = co.conrelid\n" + " AND co.contype = 'n' AND co.conkey = array[a.attnum])\n" + "WHERE a.attnum > 0::pg_catalog.int2\n" + "ORDER BY a.attrelid, a.attnum"); res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); @@ -8525,6 +8542,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_attalign = PQfnumber(res, "attalign"); i_attislocal = PQfnumber(res, "attislocal"); i_attnotnull = PQfnumber(res, "attnotnull"); + i_localnotnull = PQfnumber(res, "local_notnull"); i_attoptions = PQfnumber(res, "attoptions"); i_attcollation = PQfnumber(res, "attcollation"); i_attcompression = PQfnumber(res, "attcompression"); @@ -8533,8 +8551,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_atthasdef = PQfnumber(res, "atthasdef"); /* Within the next loop, we'll accumulate OIDs of tables with defaults */ - resetPQExpBuffer(tbloids); - appendPQExpBufferChar(tbloids, '{'); + resetPQExpBuffer(defaultoids); + appendPQExpBufferChar(defaultoids, '{'); /* * Outer loop iterates once per table, not once per row. Incrementing of @@ -8590,7 +8608,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attfdwoptions = (char **) pg_malloc(numatts * sizeof(char *)); tbinfo->attmissingval = (char **) pg_malloc(numatts * sizeof(char *)); tbinfo->notnull = (bool *) pg_malloc(numatts * sizeof(bool)); - tbinfo->inhNotNull = (bool *) pg_malloc(numatts * sizeof(bool)); + tbinfo->localNotNull = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *)); hasdefaults = false; @@ -8612,6 +8630,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attalign[j] = *(PQgetvalue(res, r, i_attalign)); tbinfo->attislocal[j] = (PQgetvalue(res, r, i_attislocal)[0] == 't'); tbinfo->notnull[j] = (PQgetvalue(res, r, i_attnotnull)[0] == 't'); + tbinfo->localNotNull[j] = (PQgetvalue(res, r, i_localnotnull)[0] == 't'); tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, r, i_attoptions)); tbinfo->attcollation[j] = atooid(PQgetvalue(res, r, i_attcollation)); tbinfo->attcompression[j] = *(PQgetvalue(res, r, i_attcompression)); @@ -8620,16 +8639,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attrdefs[j] = NULL; /* fix below */ if (PQgetvalue(res, r, i_atthasdef)[0] == 't') hasdefaults = true; - /* these flags will be set in flagInhAttrs() */ - tbinfo->inhNotNull[j] = false; } if (hasdefaults) { /* Collect OIDs of interesting tables that have defaults */ - if (tbloids->len > 1) /* do we have more than the '{'? */ - appendPQExpBufferChar(tbloids, ','); - appendPQExpBuffer(tbloids, "%u", tbinfo->dobj.catId.oid); + if (defaultoids->len > 1) /* do we have more than the '{'? */ + appendPQExpBufferChar(defaultoids, ','); + appendPQExpBuffer(defaultoids, "%u", tbinfo->dobj.catId.oid); } } @@ -8639,7 +8656,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) * Now get info about column defaults. This is skipped for a data-only * dump, as it is only needed for table schemas. */ - if (!dopt->dataOnly && tbloids->len > 1) + if (!dopt->dataOnly && defaultoids->len > 1) { AttrDefInfo *attrdefs; int numDefaults; @@ -8647,14 +8664,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) pg_log_info("finding table default expressions"); - appendPQExpBufferChar(tbloids, '}'); + appendPQExpBufferChar(defaultoids, '}'); printfPQExpBuffer(q, "SELECT a.tableoid, a.oid, adrelid, adnum, " "pg_catalog.pg_get_expr(adbin, adrelid) AS adsrc\n" "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_attrdef a ON (src.tbloid = a.adrelid)\n" "ORDER BY a.adrelid, a.adnum", - tbloids->data); + defaultoids->data); res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); @@ -8897,9 +8914,111 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) PQclear(res); } + /* + * Get info about table NOT NULL constraints. This is skipped for a + * data-only dump, as it is only needed for table schemas. + * + * Optimizing for tables that have no NOT NULL constraint seems + * pointless, so we don't try. + */ + if (!dopt->dataOnly) + { + ConstraintInfo *constrs; + int numConstrs; + int i_tableoid; + int i_oid; + int i_conrelid; + int i_conname; + int i_condef; + int i_conislocal; + + pg_log_info("finding table not null constraints"); + + /* + * Only constraints marked connoinherit need to be handled here; + * most of them are instead handled when columns are defined. + */ + resetPQExpBuffer(q); + appendPQExpBuffer(q, + "SELECT co.tableoid, co.oid, conrelid, conname, " + "pg_catalog.pg_get_constraintdef(co.oid) AS condef,\n" + " conislocal, coninhcount, connoinherit " + "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" + "JOIN pg_catalog.pg_constraint co ON (src.tbloid = co.conrelid)\n" + "JOIN pg_catalog.pg_class c ON (conrelid = c.oid)\n" + "WHERE contype = 'n' AND (connoinherit OR relispartition)\n" + "ORDER BY co.conrelid, co.conname", + tbloids->data); + + res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); + + numConstrs = PQntuples(res); + constrs = (ConstraintInfo *) pg_malloc(numConstrs * sizeof(ConstraintInfo)); + + i_tableoid = PQfnumber(res, "tableoid"); + i_oid = PQfnumber(res, "oid"); + i_conrelid = PQfnumber(res, "conrelid"); + i_conname = PQfnumber(res, "conname"); + i_condef = PQfnumber(res, "condef"); + i_conislocal = PQfnumber(res, "conislocal"); + + /* As above, this loop iterates once per table, not once per row */ + curtblindx = -1; + for (int j = 0; j < numConstrs;) + { + Oid conrelid = atooid(PQgetvalue(res, j, i_conrelid)); + TableInfo *tbinfo = NULL; + int numcons; + + /* Count rows for this table */ + for (numcons = 1; numcons < numConstrs - j; numcons++) + if (atooid(PQgetvalue(res, j + numcons, i_conrelid)) != conrelid) + break; + + /* + * Locate the associated TableInfo; we rely on tblinfo[] being in + * OID order. + */ + while (++curtblindx < numTables) + { + tbinfo = &tblinfo[curtblindx]; + if (tbinfo->dobj.catId.oid == conrelid) + break; + } + if (curtblindx >= numTables) + pg_fatal("unrecognized table OID %u", conrelid); + + for (int c = 0; c < numcons; c++, j++) + { + constrs[j].dobj.objType = DO_CONSTRAINT; + constrs[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid)); + constrs[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid)); + AssignDumpId(&constrs[j].dobj); + constrs[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname)); + constrs[j].dobj.namespace = tbinfo->dobj.namespace; + constrs[j].contable = tbinfo; + constrs[j].condomain = NULL; + constrs[j].contype = 'n'; + constrs[j].condef = pg_strdup(PQgetvalue(res, j, i_condef)); + constrs[j].confrelid = InvalidOid; + constrs[j].conindex = 0; + constrs[j].condeferrable = false; + constrs[j].condeferred = false; + constrs[j].conislocal = (PQgetvalue(res, j, i_conislocal)[0] == 't'); + + constrs[j].separate = true; + + constrs[j].dobj.dump = tbinfo->dobj.dump; + } + } + + PQclear(res); + } + destroyPQExpBuffer(q); destroyPQExpBuffer(tbloids); destroyPQExpBuffer(checkoids); + destroyPQExpBuffer(defaultoids); } /* @@ -15572,12 +15691,12 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) !tbinfo->attrdefs[j]->separate); /* - * Not Null constraint --- suppress if inherited, except - * if partition, or in binary-upgrade case where that - * won't work. + * Not Null constraint --- suppress unless it is locally + * defined, except if partition, or in binary-upgrade case + * where that won't work. */ print_notnull = (tbinfo->notnull[j] && - (!tbinfo->inhNotNull[j] || + (tbinfo->localNotNull[j] || tbinfo->ispartition || dopt->binary_upgrade)); /* @@ -15970,7 +16089,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) * we have to mark it separately. */ if (!shouldPrintColumn(dopt, tbinfo, j) && - tbinfo->notnull[j] && !tbinfo->inhNotNull[j]) + tbinfo->notnull[j] && tbinfo->localNotNull[j]) appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", foreign, qualrelname, @@ -16795,6 +16914,31 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo) .createStmt = q->data, .dropStmt = delq->data)); } + else if (coninfo->contype == 'n') + { + appendPQExpBuffer(q, "ALTER %sTABLE %s\n", foreign, + fmtQualifiedDumpable(tbinfo)); + appendPQExpBuffer(q, " ADD CONSTRAINT %s %s;\n", + fmtId(coninfo->dobj.name), + coninfo->condef); + + appendPQExpBuffer(delq, "ALTER %sTABLE %s\n", foreign, + fmtQualifiedDumpable(tbinfo)); + appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n", + fmtId(coninfo->dobj.name)); + + tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name); + + if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION) + ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId, + ARCHIVE_OPTS(.tag = tag, + .namespace = tbinfo->dobj.namespace->dobj.name, + .owner = tbinfo->rolname, + .description = "NOT NULL CONSTRAINT", + .section = SECTION_POST_DATA, + .createStmt = q->data, + .dropStmt = delq->data)); + } else if (coninfo->contype == 'c' && tbinfo) { /* CHECK constraint on a table */ diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index ed6ce41ad7..765fe6399a 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -345,7 +345,7 @@ typedef struct _tableInfo char **attfdwoptions; /* per-attribute fdw options */ char **attmissingval; /* per attribute missing value */ bool *notnull; /* NOT NULL constraints on attributes */ - bool *inhNotNull; /* true if NOT NULL is inherited */ + bool *localNotNull; /* true if NOT NULL has local definition */ struct _attrDefInfo **attrdefs; /* DEFAULT expressions */ struct _constraintInfo *checkexprs; /* CHECK constraints */ bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */ diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 93e24d5145..afd1bb2e9a 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -3115,7 +3115,7 @@ my %tests = ( );', regexp => qr/^ \QCREATE TABLE dump_test.fk_reference_test_table (\E - \n\s+\Qcol1 integer NOT NULL\E + \n\s+\Qcol1 integer\E \n\); /xm, like => @@ -3507,7 +3507,7 @@ my %tests = ( );', regexp => qr/^ \QCREATE TABLE dump_test.test_table_generated (\E\n - \s+\Qcol1 integer NOT NULL,\E\n + \s+\Qcol1 integer,\E\n \s+\Qcol2 integer GENERATED ALWAYS AS ((col1 * 2)) STORED\E\n \); /xms, @@ -3621,7 +3621,7 @@ my %tests = ( ) INHERITS (dump_test.test_inheritance_parent);', regexp => qr/^ \QCREATE TABLE dump_test.test_inheritance_child (\E\n - \s+\Qcol1 integer,\E\n + \s+\Qcol1 integer NOT NULL,\E\n \s+\QCONSTRAINT test_inheritance_child CHECK ((col2 >= 142857))\E\n \)\n \QINHERITS (dump_test.test_inheritance_parent);\E\n diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index d01ab504b6..e774124c27 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 List *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 16bf5f5576..13573a3cf1 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 17b9404937..8f7ce96651 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -73,6 +73,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 cc7b32b279..46bf610764 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2177,6 +2177,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 */ @@ -2462,10 +2463,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). * ---------------------- */ @@ -2480,6 +2482,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 */ @@ -2568,6 +2571,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..88977bf2c7 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; @@ -318,6 +321,7 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS) if (OidIsValid(sub->address.objectId)) { char *objdesc; + objdesc = getObjectDescription((const ObjectAddress *) &sub->address, false); values[1] = CStringGetTextDatum(objdesc); } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 3b708c7976..010a215cb4 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,23 +1195,10 @@ 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; -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 -DETAIL: Failing row contains (null, foo). -delete from child; -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 -DETAIL: Failing row contains (null, foo). drop table child; drop table parent; -- test setting and removing default values @@ -3834,6 +3825,28 @@ Referenced by: TABLE "ataddindex" CONSTRAINT "ataddindex_ref_id_fkey" FOREIGN KEY (ref_id) REFERENCES ataddindex(id) DROP TABLE ataddindex; +CREATE TABLE atnotnull1 (); +ALTER TABLE atnotnull1 + ADD COLUMN a INT, + ALTER a SET NOT NULL; +ALTER TABLE atnotnull1 + ADD COLUMN b INT, + ADD NOT NULL b; +ALTER TABLE atnotnull1 + ADD COLUMN c INT, + ADD PRIMARY KEY (c); +SELECT conrelid::regclass, conname, contype, conkey, + (SELECT attname FROM pg_attribute WHERE attrelid = conrelid AND attnum = conkey[1]), + coninhcount, conislocal + FROM pg_constraint WHERE contype IN ('n','p') AND + conrelid IN ('atnotnull1'::regclass); + conrelid | conname | contype | conkey | attname | coninhcount | conislocal +------------+-----------------------+---------+--------+---------+-------------+------------ + atnotnull1 | atnotnull1_a_not_null | n | {1} | a | 0 | t + atnotnull1 | atnotnull1_b_not_null | n | {2} | b | 0 | t + atnotnull1 | atnotnull1_pkey | p | {3} | c | 0 | t +(3 rows) + -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( a int, @@ -4355,8 +4368,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..014205b6bf 100644 --- a/src/test/regress/expected/constraints.out +++ b/src/test/regress/expected/constraints.out @@ -288,6 +288,28 @@ ERROR: new row for relation "atacc1" violates check constraint "atacc1_test2_ch DETAIL: Failing row contains (null, 3). DROP TABLE ATACC1 CASCADE; NOTICE: drop cascades to table atacc2 +-- NOT NULL NO INHERIT +CREATE TABLE ATACC1 (a int, not null a no inherit); +CREATE TABLE ATACC2 () INHERITS (ATACC1); +\d ATACC2 + Table "public.atacc2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Inherits: atacc1 + +DROP TABLE ATACC1, ATACC2; +CREATE TABLE ATACC1 (a int); +ALTER TABLE ATACC1 ADD NOT NULL a NO INHERIT; +CREATE TABLE ATACC2 () INHERITS (ATACC1); +\d ATACC2 + Table "public.atacc2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Inherits: atacc1 + +DROP TABLE ATACC1, ATACC2; -- -- Check constraints on INSERT INTO -- @@ -754,6 +776,98 @@ 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 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 | + +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; + conname | contype | conkey +-------------------------+---------+-------- + notnull_tbl1_a_not_null | n | {1} +(1 row) + +-- 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 | | | + +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; + conname | contype | conkey +---------+---------+-------- +(0 rows) + +-- 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 | + +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; + conname | contype | conkey +-------------------------+---------+-------- + notnull_tbl1_a_not_null | n | {1} +(1 row) + +-- Doing it twice doesn't create a redundant constraint +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; + conname | contype | conkey +-------------------------+---------+-------- + notnull_tbl1_a_not_null | n | {1} +(1 row) + +-- Using the "table constraint" syntax also works +ALTER TABLE notnull_tbl1 ALTER a DROP NOT NULL; +ALTER TABLE notnull_tbl1 ADD CONSTRAINT foobar NOT NULL a; +\d notnull_tbl1 + Table "public.notnull_tbl1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | + +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; + conname | contype | conkey +---------+---------+-------- + foobar | n | {1} +(1 row) + +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/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 55f7158c1a..a601f33268 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2036,13 +2036,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; @@ -2065,13 +2071,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 a7fbeed9eb..8fcec04cb5 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1847,6 +1847,403 @@ 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, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | conkey | attname | coninhcount | conislocal +----------+-----------------+---------+--------+---------+-------------+------------ + cc1 | nn | n | {4} | a2 | 0 | t + cc2 | nn | n | {5} | a2 | 1 | f + pp1 | pp1_f1_not_null | n | {1} | f1 | 0 | t + cc1 | pp1_f1_not_null | n | {1} | f1 | 1 | f + cc2 | pp1_f1_not_null | n | {1} | f1 | 1 | f +(5 rows) + +-- remove constraint from cc2: no dice, it's inherited +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, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | conkey | attname | coninhcount | conislocal +----------+-----------------+---------+--------+---------+-------------+------------ + pp1 | pp1_f1_not_null | n | {1} | f1 | 0 | t + cc1 | pp1_f1_not_null | n | {1} | f1 | 1 | f + cc2 | pp1_f1_not_null | n | {1} | f1 | 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; +-- have a look at pg_constraint +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid in ('pp1'::regclass, 'cc1'::regclass, 'cc2'::regclass) + order by 2, 1; + conrelid | conname | contype | conkey | attname | coninhcount | conislocal +----------+---------+---------+--------+---------+-------------+------------ +(0 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 "dropping" a not null constraint that's also inherited +create table inh_parent (a int not null); +create table inh_child (a int not null) inherits (inh_parent); +NOTICE: merging column "a" with inherited definition +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); + conrelid | conname | contype | conkey | attname | coninhcount | conislocal | connoinherit +------------+-----------------------+---------+--------+---------+-------------+------------+-------------- + inh_parent | inh_parent_a_not_null | n | {1} | a | 0 | t | f + inh_child | inh_child_a_not_null | n | {1} | a | 1 | t | f +(2 rows) + +alter table inh_child alter a drop not null; +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); + conrelid | conname | contype | conkey | attname | coninhcount | conislocal | connoinherit +------------+-----------------------+---------+--------+---------+-------------+------------+-------------- + inh_parent | inh_parent_a_not_null | n | {1} | a | 0 | t | f + inh_child | inh_child_a_not_null | n | {1} | a | 1 | f | f +(2 rows) + +alter table inh_parent alter a drop not null; +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); + conrelid | conname | contype | conkey | attname | coninhcount | conislocal | connoinherit +----------+---------+---------+--------+---------+-------------+------------+-------------- +(0 rows) + +drop table inh_parent, inh_child; +-- NOT NULL NO INHERIT +create table inh_parent(a int); +create table inh_child() inherits (inh_parent); +alter table inh_parent add not null a no inherit; +create table inh_child2() inherits (inh_parent); +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child'::regclass, 'inh_child2'::regclass) + order by 2, 1; + conrelid | conname | contype | conkey | attname | coninhcount | conislocal | connoinherit +------------+-----------------------+---------+--------+---------+-------------+------------+-------------- + inh_parent | inh_parent_a_not_null | n | {1} | a | 0 | t | t +(1 row) + +\d inh_parent + Table "public.inh_parent" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | not null | +Number of child tables: 2 (Use \d+ to list them.) + +\d inh_child + Table "public.inh_child" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Inherits: inh_parent + +\d inh_child2 + Table "public.inh_child2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | +Inherits: inh_parent + +drop table inh_parent, inh_child, inh_child2; +-- +-- 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 | 1 | t + inh_child2 | inh_child2_f1_not_null | n | 1 | 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 | 1 | 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 explicitly 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; +-- -- 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/parallel_schedule b/src/test/regress/parallel_schedule index 3624035639..bae480dabf 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -121,7 +121,8 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion tr # ---------- test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort explain compression memoize stats -# event_trigger cannot run concurrently with any test that runs DDL +# event_trigger depends on create_am and cannot run concurrently with +# any test that runs DDL # oidjoins is read-only, though, and should run late for best coverage test: event_trigger oidjoins diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 58ea20ac3d..4f7003523a 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; @@ -917,14 +917,6 @@ insert into parent values (NULL); insert into child (a, b) values (NULL, 'foo'); alter table only parent alter a set not null; alter table child alter a set not null; -delete from parent; -alter table only parent alter a set not null; -insert into parent values (NULL); -alter table child alter a set not null; -insert into child (a, b) values (NULL, 'foo'); -delete from child; -alter table child alter a set not null; -insert into child (a, b) values (NULL, 'foo'); drop table child; drop table parent; @@ -2342,6 +2334,22 @@ ALTER TABLE ataddindex \d ataddindex DROP TABLE ataddindex; +CREATE TABLE atnotnull1 (); +ALTER TABLE atnotnull1 + ADD COLUMN a INT, + ALTER a SET NOT NULL; +ALTER TABLE atnotnull1 + ADD COLUMN b INT, + ADD NOT NULL b; +ALTER TABLE atnotnull1 + ADD COLUMN c INT, + ADD PRIMARY KEY (c); +SELECT conrelid::regclass, conname, contype, conkey, + (SELECT attname FROM pg_attribute WHERE attrelid = conrelid AND attnum = conkey[1]), + coninhcount, conislocal + FROM pg_constraint WHERE contype IN ('n','p') AND + conrelid IN ('atnotnull1'::regclass); + -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( a int, diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql index 5ffcd4ffc7..5a3c904660 100644 --- a/src/test/regress/sql/constraints.sql +++ b/src/test/regress/sql/constraints.sql @@ -196,6 +196,17 @@ INSERT INTO ATACC2 (TEST2) VALUES (3); INSERT INTO ATACC1 (TEST2) VALUES (3); DROP TABLE ATACC1 CASCADE; +-- NOT NULL NO INHERIT +CREATE TABLE ATACC1 (a int, not null a no inherit); +CREATE TABLE ATACC2 () INHERITS (ATACC1); +\d ATACC2 +DROP TABLE ATACC1, ATACC2; +CREATE TABLE ATACC1 (a int); +ALTER TABLE ATACC1 ADD NOT NULL a NO INHERIT; +CREATE TABLE ATACC2 () INHERITS (ATACC1); +\d ATACC2 +DROP TABLE ATACC1, ATACC2; + -- -- Check constraints on INSERT INTO -- @@ -556,6 +567,38 @@ ALTER TABLE deferred_excl ADD EXCLUDE (f1 WITH =); DROP TABLE deferred_excl; +-- verify constraints created for NOT NULL clauses +CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL); +\d notnull_tbl1 +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; +-- 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 +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; +-- SET NOT NULL puts both back +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +\d notnull_tbl1 +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; +-- Doing it twice doesn't create a redundant constraint +ALTER TABLE notnull_tbl1 ALTER a SET NOT NULL; +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; +-- Using the "table constraint" syntax also works +ALTER TABLE notnull_tbl1 ALTER a DROP NOT NULL; +ALTER TABLE notnull_tbl1 ADD CONSTRAINT foobar NOT NULL a; +\d notnull_tbl1 +select conname, contype, conkey from pg_constraint where conrelid = 'notnull_tbl1'::regclass; +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/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 215d58e80d..9cb897e1b4 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -679,6 +679,213 @@ 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, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + 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: no dice, it's inherited +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, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + 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, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + 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 "dropping" a not null constraint that's also inherited +create table inh_parent (a int not null); +create table inh_child (a int not null) inherits (inh_parent); +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); +alter table inh_child alter a drop not null; +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); +alter table inh_parent alter a drop not null; +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype in ('n','p') and + conrelid in ('inh_child'::regclass, 'inh_parent'::regclass); +drop table inh_parent, inh_child; + +-- NOT NULL NO INHERIT +create table inh_parent(a int); +create table inh_child() inherits (inh_parent); +alter table inh_parent add not null a no inherit; +create table inh_child2() inherits (inh_parent); +select conrelid::regclass, conname, contype, conkey, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal, connoinherit + from pg_constraint where contype = 'n' and + conrelid in ('inh_parent'::regclass, 'inh_child'::regclass, 'inh_child2'::regclass) + order by 2, 1; +\d inh_parent +\d inh_child +\d inh_child2 +drop table inh_parent, inh_child, inh_child2; + +-- +-- 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 explicitly 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; + -- -- 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.39.2