From 0358b6984bb52947f085d5388bfbf0035a1dd631 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 30 Jun 2023 13:36:24 +0200 Subject: [PATCH v12] Add pg_constraint rows for NOT NULL constraints --- contrib/sepgsql/expected/alter.out | 3 - contrib/sepgsql/expected/ddl.out | 2 + doc/src/sgml/catalogs.sgml | 1 + doc/src/sgml/ref/alter_table.sgml | 11 +- doc/src/sgml/ref/create_table.sgml | 8 +- src/backend/catalog/heap.c | 493 ++++-- src/backend/catalog/pg_constraint.c | 105 +- src/backend/commands/tablecmds.c | 1444 ++++++++++++----- src/backend/nodes/outfuncs.c | 4 + src/backend/nodes/readfuncs.c | 8 +- src/backend/optimizer/util/plancat.c | 2 + src/backend/parser/gram.y | 19 +- src/backend/parser/parse_utilcmd.c | 279 +++- src/backend/utils/adt/ruleutils.c | 14 + src/backend/utils/cache/relcache.c | 21 +- src/bin/pg_dump/common.c | 18 +- src/bin/pg_dump/pg_backup_archiver.c | 2 + src/bin/pg_dump/pg_dump.c | 290 +++- src/bin/pg_dump/pg_dump.h | 9 +- src/bin/pg_dump/t/002_pg_dump.pl | 10 +- src/include/catalog/heap.h | 8 +- 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 | 26 +- .../test_ddl_deparse/test_ddl_deparse.c | 6 +- src/test/regress/expected/alter_table.out | 61 +- src/test/regress/expected/cluster.out | 7 +- src/test/regress/expected/constraints.out | 117 ++ src/test/regress/expected/create_table.out | 35 +- 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 | 408 +++++ .../regress/expected/replica_identity.out | 16 + src/test/regress/parallel_schedule | 3 +- src/test/regress/sql/alter_table.sql | 28 +- src/test/regress/sql/constraints.sql | 46 + src/test/regress/sql/create_table.sql | 6 +- src/test/regress/sql/indexing.sql | 8 +- src/test/regress/sql/inherit.sql | 211 +++ src/test/regress/sql/replica_identity.sql | 15 + 44 files changed, 3128 insertions(+), 731 deletions(-) diff --git a/contrib/sepgsql/expected/alter.out b/contrib/sepgsql/expected/alter.out index c604cc7768..510b2ded52 100644 --- a/contrib/sepgsql/expected/alter.out +++ b/contrib/sepgsql/expected/alter.out @@ -164,7 +164,6 @@ LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_re LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_table_2.b" permissive=0 ALTER TABLE regtest_table ALTER b DROP NOT NULL; LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_table.b" permissive=0 -LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_table_2.b" permissive=0 ALTER TABLE regtest_table ALTER b SET STATISTICS -1; LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_table.b" permissive=0 LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_table_2.b" permissive=0 @@ -245,8 +244,6 @@ LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_re LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_ptable_1_tens.p" permissive=0 ALTER TABLE regtest_ptable ALTER p DROP NOT NULL; LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_ptable.p" permissive=0 -LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_table_part.p" permissive=0 -LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_ptable_1_tens.p" permissive=0 ALTER TABLE regtest_ptable ALTER p SET STATISTICS -1; LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_ptable.p" permissive=0 LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema_2.regtest_table_part.p" permissive=0 diff --git a/contrib/sepgsql/expected/ddl.out b/contrib/sepgsql/expected/ddl.out index 15d2b9c5e7..70bd6525c0 100644 --- a/contrib/sepgsql/expected/ddl.out +++ b/contrib/sepgsql/expected/ddl.out @@ -49,6 +49,7 @@ LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_reg LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=system_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="pg_catalog" permissive=0 LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 +LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 LOG: SELinux: allowed { add_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_table name="regtest_schema.regtest_table" permissive=0 LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 @@ -269,6 +270,7 @@ LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_reg LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_table_4.y" permissive=0 LOG: SELinux: allowed { create } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_column name="regtest_schema.regtest_table_4.z" permissive=0 LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 +LOG: SELinux: allowed { search } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 LOG: SELinux: allowed { add_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0 tclass=db_schema name="regtest_schema" permissive=0 LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0 tclass=db_table name="regtest_schema.regtest_table_4" permissive=0 CREATE INDEX regtest_index_tbl4_y ON regtest_table_4(y); diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 852cb30ae1..1951ee05e3 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..2c4138e4e9 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -113,6 +113,7 @@ WITH ( MODULUS numeric_literal, REM [ CONSTRAINT constraint_name ] { CHECK ( expression ) [ NO INHERIT ] | + NOT NULL column_name [ NO INHERIT ] | 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 ) ] | @@ -1763,11 +1764,17 @@ ALTER TABLE measurement Compatibility - The forms ADD (without USING INDEX), + The forms ADD [COLUMN], DROP [COLUMN], DROP IDENTITY, RESTART, SET DEFAULT, SET DATA TYPE (without USING), SET GENERATED, and SET sequence_option - conform with the SQL standard. The other forms are + conform with the SQL standard. + The form ADD table_constraint + conforms with the SQL standard when the USING INDEX and + NOT VALID clauses are omitted and the constraint type is + one of CHECK, UNIQUE, PRIMARY KEY, + or REFERENCES. + The other forms are PostgreSQL extensions of the SQL standard. Also, the ability to specify more than one manipulation in a single ALTER TABLE command is an extension. diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 10ef699fab..e04a0692c4 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 [ NO INHERIT ] | 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..2a8a39030c 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, int 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,174 @@ 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. - */ - 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 +2741,191 @@ 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 + * flag set. + */ +List * +AddRelationNotNullConstraints(Relation rel, List *constraints, + List *old_notnulls) +{ + List *nnnames = NIL; + List *givennames = NIL; + List *nncols = NIL; + 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), constr->conname) == 0) + ereport(ERROR, + errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("constraint \"%s\" for relation \"%s\" already exists", + 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 old_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, deleting elements we've already + * processed. + * + * We don't use foreach() here because we have two nested loops over the + * cooked constraint list, with possible element deletions in the inner + * one. If we used foreach_delete_current() it could only fix up the state + * of one of the loops, so it seems cleaner to use looping over list + * indexes for both loops. Note that any deletion will happen beyond + * where the outer loop is, so its index never needs adjustment. + */ + list_sort(old_notnulls, list_cookedconstr_attnum_cmp); + for (int outerpos = 0; outerpos < list_length(old_notnulls); outerpos++) + { + CookedConstraint *cooked; + char *conname = NULL; + int inhcount = 1; + ListCell *lc2; + + cooked = (CookedConstraint *) list_nth(old_notnulls, outerpos); + + /* We just preserve the first constraint name we come across, if any */ + if (conname == NULL && cooked->name) + conname = cooked->name; + + for (int restpos = outerpos + 1; restpos < list_length(old_notnulls);) + { + CookedConstraint *other; + + other = (CookedConstraint *) list_nth(old_notnulls, restpos); + if (other->attnum == cooked->attnum) + { + if (conname == NULL && other->name) + conname = other->name; + + inhcount++; + old_notnulls = list_delete_nth_cell(old_notnulls, restpos); + } + else + restpos++; + } + + /* 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); + } + + 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..844f1a641b 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -562,6 +562,103 @@ 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) +{ + AttrNumber colnum; + Datum adatum; + ArrayType *arr; + + /* only tuples for NOT NULL constraints should be given */ + Assert(((Form_pg_constraint) GETSTRUCT(constrTup))->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. */ @@ -1129,7 +1226,6 @@ get_primary_key_attnos(Oid relid, bool deferrableOk, Oid *constraintOid) { Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); Datum adatum; - bool isNull; ArrayType *arr; int16 *attnums; int numkeys; @@ -1148,11 +1244,8 @@ get_primary_key_attnos(Oid relid, bool deferrableOk, Oid *constraintOid) break; /* Extract the conkey array, ie, attnums of PK's columns */ - adatum = heap_getattr(tuple, Anum_pg_constraint_conkey, - RelationGetDescr(pg_constraint), &isNull); - if (isNull) - elog(ERROR, "null conkey for constraint %u", - ((Form_pg_constraint) GETSTRUCT(tuple))->oid); + adatum = SysCacheGetAttrNotNull(CONSTROID, tuple, + Anum_pg_constraint_conkey); arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ numkeys = ARR_DIMS(arr)[0]; if (ARR_NDIM(arr) != 1 || diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8fff036b73..4ca82f3879 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -351,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 **supnotnulls); 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); @@ -432,16 +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 void ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel, - const char *colName, LOCKMODE lockmode); +static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, bool recurse, + LOCKMODE lockmode); +static bool 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 ObjectAddress ATExecSetAttNotNull(List **wqueue, Relation rel, + const char *colName, LOCKMODE lockmode); static bool NotNullImpliedByRelConstraints(Relation rel, Form_pg_attribute attr); static bool ConstraintImpliedByRelConstraint(Relation scanrel, List *testConstraint, List *provenConstraint); @@ -481,11 +482,11 @@ static ObjectAddress ATExecAddConstraint(List **wqueue, static char *ChooseForeignKeyConstraintNameAddition(List *colnames); static ObjectAddress ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel, IndexStmt *stmt, LOCKMODE lockmode); -static ObjectAddress ATAddCheckConstraint(List **wqueue, - AlteredTableInfo *tab, Relation rel, - Constraint *constr, - bool recurse, bool recursing, bool is_readd, - LOCKMODE lockmode); +static ObjectAddress ATAddCheckNNConstraint(List **wqueue, + AlteredTableInfo *tab, Relation rel, + Constraint *constr, + bool recurse, bool recursing, bool is_readd, + LOCKMODE lockmode); static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *fkconstraint, bool recurse, bool recursing, @@ -542,6 +543,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, @@ -617,7 +623,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); @@ -635,6 +641,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); @@ -672,8 +679,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; @@ -863,12 +872,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); @@ -1251,6 +1261,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); /* @@ -2299,6 +2320,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. + * 'supnotnulls' receives a list of CookedConstraints that corresponds to + * constraints coming from inheritance parents. * * Return value: * Completed schema list. @@ -2329,7 +2352,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. @@ -2348,10 +2374,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 */ @@ -2463,9 +2490,12 @@ 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); @@ -2554,6 +2584,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++) { @@ -2649,9 +2693,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 @@ -2685,7 +2758,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; @@ -2702,6 +2779,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); + } } /* @@ -2846,6 +2950,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); /* @@ -3052,8 +3169,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. Also, merge column defaults. */ if (is_partition) { @@ -3070,7 +3186,6 @@ MergeAttributes(List *schema, List *supers, char relpersistence, if (strcmp(coldef->colname, restdef->colname) == 0) { found = true; - coldef->is_not_null |= restdef->is_not_null; /* * Check for conflicts related to generated columns. @@ -3159,6 +3274,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence, } *supconstr = constraints; + *supnotnulls = nnconstraints; + return schema; } @@ -3210,6 +3327,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 @@ -3770,7 +3966,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) { @@ -4355,6 +4554,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: @@ -4493,15 +4693,6 @@ AlterTableGetLockLevel(List *cmds) cmd_lockmode = ShareUpdateExclusiveLock; break; - case AT_CheckNotNull: - - /* - * This only examines the table's schema; but lock must be - * strong enough to prevent concurrent DROP NOT NULL. - */ - cmd_lockmode = AccessShareLock; - break; - default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", (int) cmd->subtype); @@ -4653,21 +4844,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_CheckNotNull: /* check column is already marked NOT NULL */ + 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); - /* No command-specific prep needed */ pass = AT_PASS_COL_ATTRS; break; case AT_DropExpression: /* ALTER COLUMN DROP EXPRESSION */ @@ -5046,13 +5239,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_CheckNotNull: /* check column is already marked NOT NULL */ - ATExecCheckNotNull(tab, rel, cmd->name, lockmode); + case AT_SetAttNotNull: /* set pg_attribute.attnotnull */ + address = ATExecSetAttNotNull(wqueue, rel, cmd->name, lockmode); break; case AT_DropExpression: address = ATExecDropExpression(rel, cmd->name, cmd->missing_ok, lockmode); @@ -5388,11 +5582,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: @@ -6068,6 +6259,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; @@ -6176,10 +6368,10 @@ 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: - return NULL; /* not real grammar */ case AT_SetStatistics: return "ALTER COLUMN ... SET STATISTICS"; case AT_SetOptions: @@ -6775,8 +6967,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) { @@ -7291,41 +7482,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; /* @@ -7341,6 +7510,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) @@ -7356,62 +7534,37 @@ 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); @@ -7429,19 +7582,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; + + dropconstraint_internal(rel, conTup, DROP_RESTRICT, recurse, false, + false, NULL, lockmode); + + heap_freetuple(conTup); InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), attnum); @@ -7452,102 +7619,134 @@ 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. + * + * Returns true if the flag was set in any table, otherwise false. */ - -static void -ATPrepSetNotNull(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode, AlterTableUtilityContext *context) +static bool +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) - return; + HeapTuple tuple; + Form_pg_attribute attForm; + bool retval = false; - /* - * 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) + 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) { - HeapTuple tuple; - bool attnotnull; + Relation attr_rel; - tuple = SearchSysCacheAttName(RelationGetRelid(rel), cmd->name); + attr_rel = table_open(AttributeRelationId, RowExclusiveLock); - /* 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)))); + attForm->attnotnull = true; + CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple); - attnotnull = ((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull; - ReleaseSysCache(tuple); - if (attnotnull) - return; + 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; + } + + retval = true; } - /* - * 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) + if (recurse) { - AlterTableCmd *newcmd = makeNode(AlterTableCmd); + List *children; + ListCell *lc; - newcmd->subtype = AT_CheckNotNull; - newcmd->name = pstrdup(cmd->name); - ATSimpleRecursion(wqueue, rel, newcmd, true, lockmode, context); + children = find_inheritance_children(RelationGetRelid(rel), lockmode); + foreach(lc, children) + { + Oid childrelid = lfirst_oid(lc); + Relation childrel; + AttrNumber childattno; + + /* find_inheritance_children already got lock */ + childrel = table_open(childrelid, NoLock); + CheckTableNotInUse(childrel, "ALTER TABLE"); + + childattno = get_attnum(RelationGetRelid(childrel), + get_attname(RelationGetRelid(rel), attnum, + false)); + retval |= set_attnotnull(wqueue, childrel, childattno, + recurse, lockmode); + table_close(childrel, NoLock); + } } - else - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); + + return retval; } /* - * 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; + bool is_no_inherit = false; + 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, @@ -7555,80 +7754,177 @@ 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; + + systable_endscan(conscan); + table_close(constr_rel, RowExclusiveLock); + + /* + * If we're asked not to recurse, and children exist, raise an error for + * partitioned tables. For inheritance, we act as if NO INHERIT had been + * specified. + */ + 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("constraint must be added to child tables too"), + errhint("Do not specify the ONLY keyword.")); + else + is_no_inherit = true; + } + + /* + * 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->is_no_inherit = is_no_inherit; + 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); InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), attnum); - table_close(attr_rel, RowExclusiveLock); + /* + * 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 CHECK NOT NULL + * ALTER TABLE ALTER COLUMN SET ATTNOTNULL * - * This doesn't exist in the grammar, but we generate AT_CheckNotNull - * commands against the partitions of a partitioned table if the user - * writes ALTER TABLE ONLY ... SET NOT NULL on the partitioned table, - * or tries to create a primary key on it (which internally creates - * AT_SetNotNull on the partitioned table). Such a command doesn't - * allow us to actually modify any partition, but we want to let it - * go through if the partitions are already properly marked. - * - * In future, this might need to adjust the child table's state, likely - * by incrementing an inheritance count for the attnotnull constraint. - * For now we need only check for the presence of the flag. + * 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 -ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel, - const char *colName, LOCKMODE lockmode) +static ObjectAddress +ATExecSetAttNotNull(List **wqueue, Relation rel, + const char *colName, LOCKMODE lockmode) { - HeapTuple tuple; + AttrNumber attnum; + ObjectAddress address = InvalidObjectAddress; - tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName); - - 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)))); + errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + colName, RelationGetRelationName(rel))); - if (!((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull) - ereport(ERROR, - (errcode(ERRCODE_INVALID_TABLE_DEFINITION), - errmsg("constraint must be added to child tables too"), - errdetail("Column \"%s\" of relation \"%s\" is not already NOT NULL.", - colName, RelationGetRelationName(rel)), - errhint("Do not specify the ONLY keyword."))); + /* + * Make the change, if necessary, and only if so report the column as + * changed + */ + if (set_attnotnull(wqueue, rel, attnum, false, lockmode)) + ObjectAddressSubSet(address, RelationRelationId, + RelationGetRelid(rel), attnum); - ReleaseSysCache(tuple); + return address; } /* @@ -8873,17 +9169,18 @@ 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, - lockmode); + ATAddCheckNNConstraint(wqueue, tab, rel, + newConstraint, recurse, false, is_readd, + lockmode); break; case CONSTR_FOREIGN: @@ -8964,9 +9261,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. * @@ -8979,9 +9276,9 @@ ChooseForeignKeyConstraintNameAddition(List *colnames) * the parent table and pass that down. */ static ObjectAddress -ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, - Constraint *constr, bool recurse, bool recursing, - bool is_readd, LOCKMODE lockmode) +ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, + Constraint *constr, bool recurse, bool recursing, + bool is_readd, LOCKMODE lockmode) { List *newcons; ListCell *lcon; @@ -9019,7 +9316,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, { CookedConstraint *ccon = (CookedConstraint *) lfirst(lcon); - if (!ccon->skip_validation) + if (!ccon->skip_validation && ccon->contype != CONSTR_NOTNULL) { NewConstraint *newcon; @@ -9035,6 +9332,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); } @@ -9090,9 +9395,13 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, /* Find or create work queue entry for this table */ childtab = ATGetQueueEntry(wqueue, childrel); - /* Recurse to child */ - ATAddCheckConstraint(wqueue, childtab, childrel, - constr, recurse, true, is_readd, lockmode); + /* + * Recurse to child. XXX if we didn't create a constraint on the + * parent because it already existed, and we do create one on a child, + * should we return that child's constraint ObjectAddress here? + */ + ATAddCheckNNConstraint(wqueue, childtab, childrel, + constr, recurse, true, is_readd, lockmode); table_close(childrel, NoLock); } @@ -11959,16 +12268,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) @@ -11997,47 +12301,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; } @@ -12046,31 +12311,249 @@ 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; + 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); } /* - * For partitioned tables, non-CHECK inherited constraints are dropped via - * the dependency mechanism, so we're done here. + * Perform the actual constraint deletion */ - if (contype != CONSTRAINT_CHECK && + 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 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 NOT NULL constraint for a column in + * the replica identity index, either. (FULL is not affected.) + */ + 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, non-NOT-NULL inherited constraints + * are dropped via the dependency mechanism, so we're done here. + */ + if (con->contype != CONSTRAINT_CHECK && + con->contype != CONSTRAINT_NOTNULL && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { table_close(conrel, RowExclusiveLock); - return; + return conobj; } /* @@ -12095,50 +12578,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); @@ -12148,17 +12685,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 */ @@ -12172,8 +12709,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); @@ -12187,6 +12724,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, } table_close(conrel, RowExclusiveLock); + + return conobj; } /* @@ -13263,9 +13802,10 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) /* * If the constraint is inherited (only), we don't want to inject a - * new definition here; it'll get recreated when ATAddCheckConstraint - * recurses from adding the parent table's constraint. But we had to - * carry the info this far so that we can drop the constraint below. + * new definition here; it'll get recreated when + * ATAddCheckNNConstraint recurses from adding the parent table's + * constraint. But we had to carry the info this far so that we can + * drop the constraint below. */ if (!conislocal) continue; @@ -13512,10 +14052,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 @@ -15259,6 +15799,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); @@ -15272,7 +15813,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); @@ -15284,7 +15825,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 */ @@ -15304,22 +15846,50 @@ 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\"", RelationGetRelationName(child_rel), NameStr(parent_con->conname)))); - /* If the child constraint is "no inherit" then cannot merge */ - if (child_con->connoinherit) + /* + * If the child constraint is "no inherit" then cannot merge. + * + * This is not desirable for NOT NULL constraints, mostly because + * it breaks our pg_upgrade strategy, but it also makes sense on + * its own: if a child has its own NOT NULL constraint and then + * acquires a parent with the same constraint, then we start to + * enforce that constraint for all the descendants of that child + * too, if any. XXX since pg_upgrade only needs this for + * inheritance and not partitioning, maybe we should also restrict + * this behavior to that case? + */ + if (child_con->contype == CONSTRAINT_CHECK && + child_con->connoinherit) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("constraint \"%s\" conflicts with non-inherited constraint on child table \"%s\"", @@ -15348,6 +15918,9 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) ereport(ERROR, errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("too many inheritance parents")); + if (child_con->contype == CONSTRAINT_NOTNULL && + child_con->connoinherit) + child_con->connoinherit = false; /* * In case of partitions, an inherited constraint must be @@ -15519,6 +16092,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; @@ -15589,6 +16163,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], @@ -15599,6 +16175,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))) { @@ -15606,6 +16183,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); @@ -15621,21 +16200,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) { @@ -17898,7 +18496,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); @@ -18011,13 +18609,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; @@ -18033,14 +18630,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++; } /* @@ -18106,7 +18702,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) * the first matching, valid, 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; @@ -18166,6 +18762,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, @@ -18178,7 +18796,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); @@ -18809,8 +19427,8 @@ DetachAddConstraintIfNeeded(List **wqueue, Relation partRel) n->initially_valid = true; n->skip_validation = true; /* It's a re-add, since it nominally already exists */ - ATAddCheckConstraint(wqueue, tab, partRel, n, - true, false, true, ShareUpdateExclusiveLock); + ATAddCheckNNConstraint(wqueue, tab, partRel, n, + true, false, true, ShareUpdateExclusiveLock); } } @@ -19079,6 +19697,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)) @@ -19215,6 +19840,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 955286513d..816ddbcd78 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -718,6 +718,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 97e43cbb49..078318017f 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 39932d3c2d..243c8fb1e4 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 edb6c00ece..20dc3f1098 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -3836,12 +3836,15 @@ ColConstraint: * or be part of a_expr NOT LIKE or similar constructs). */ ColConstraintElem: - NOT NULL_P + NOT NULL_P opt_no_inherit { Constraint *n = makeNode(Constraint); n->contype = CONSTR_NOTNULL; n->location = @1; + n->is_no_inherit = $3; + n->skip_validation = false; + n->initially_valid = true; $$ = (Node *) n; } | NULL_P @@ -4078,6 +4081,20 @@ 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; + /* no NOT VALID support yet */ + processCASbits($4, @4, "NOT NULL", + NULL, NULL, NULL, + &n->is_no_inherit, yyscanner); + n->initially_valid = true; + $$ = (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 d67580fc77..65acadbac3 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -81,6 +81,7 @@ typedef struct bool isalter; /* true if altering existing table */ List *columns; /* ColumnDef items */ List *ckconstraints; /* CHECK constraints */ + List *nnconstraints; /* NOT NULL constraints */ List *fkconstraints; /* FOREIGN KEY constraints */ List *ixconstraints; /* index-creating constraints */ List *likeclauses; /* LIKE clauses that need post-processing */ @@ -240,6 +241,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) cxt.isalter = false; cxt.columns = NIL; cxt.ckconstraints = NIL; + cxt.nnconstraints = NIL; cxt.fkconstraints = NIL; cxt.ixconstraints = NIL; cxt.likeclauses = NIL; @@ -346,6 +348,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); @@ -535,6 +538,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) bool saw_default; bool saw_identity; bool saw_generated; + bool need_notnull = false; ListCell *clist; cxt->columns = lappend(cxt->columns, column); @@ -632,10 +636,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... */ @@ -653,7 +655,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\"", @@ -665,15 +667,45 @@ 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; + + /* + * Disallow duplicate and redundant [NOT] NULL markings + */ + 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, add the constraint entry; and get rid of any + * previous markings to mark the column NOT NULL. + */ + if (!column->is_not_null) + { + column->is_not_null = true; + saw_nullable = true; + + constraint->colname = column->colname; + cxt->nnconstraints = lappend(cxt->nnconstraints, constraint); + + /* Don't need this anymore, if we had it */ + need_notnull = false; + } + break; case CONSTR_DEFAULT: @@ -723,16 +755,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; } @@ -838,6 +873,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. @@ -913,6 +971,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, @@ -924,7 +986,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: @@ -960,6 +1021,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla AclResult aclresult; char *comment; ParseCallbackState pcbstate; + bool process_notnull_constraints = false; setup_parser_errposition_callback(&pcbstate, cxt->pstate, table_like_clause->relation->location); @@ -1032,7 +1094,8 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla * Create a new column, which is marked as NOT inherited. * * For constraints, ONLY the NOT NULL constraint is inherited by the - * new column definition per SQL99. + * new column definition per SQL99; however we cannot do that + * correctly here, so we leave it for expandTableLikeClause to handle. */ def = makeNode(ColumnDef); def->colname = pstrdup(attributeName); @@ -1040,7 +1103,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla attribute->atttypmod); def->inhcount = 0; def->is_local = true; - def->is_not_null = attribute->attnotnull; + def->is_not_null = false; + if (attribute->attnotnull) + process_notnull_constraints = true; def->is_from_type = false; def->storage = 0; def->raw_default = NULL; @@ -1122,19 +1187,66 @@ 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 & - (CREATE_TABLE_LIKE_DEFAULTS | - CREATE_TABLE_LIKE_GENERATED | - CREATE_TABLE_LIKE_CONSTRAINTS | - CREATE_TABLE_LIKE_INDEXES)) + if ((table_like_clause->options & + (CREATE_TABLE_LIKE_DEFAULTS | + CREATE_TABLE_LIKE_GENERATED | + CREATE_TABLE_LIKE_CONSTRAINTS | + CREATE_TABLE_LIKE_INDEXES)) || + process_notnull_constraints) { table_like_clause->relationOid = RelationGetRelid(relation); cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause); } + /* + * However, if INCLUDING INDEXES is not given and a primary key exists, + * then we can add the necessary NOT NULL constraints for the columns + * therein. + */ + if ((table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) == 0) + { + Bitmapset *pkcols; + int x = -1; + + + pkcols = RelationGetIndexAttrBitmap(relation, + INDEX_ATTR_BITMAP_PRIMARY_KEY); + + /* + * When INCLUDING CONSTRAINTS is not specified, and the table has a + * primary key, we need to add NOT NULL constraints to cover all the + * columns in the PK. This is for backwards compatibility. + */ + while ((x = bms_next_member(pkcols, x)) >= 0) + { + Constraint *notnull; + Form_pg_attribute attForm; + + attForm = TupleDescAttr(tupleDesc, + x + FirstLowInvalidHeapAttributeNumber - 1); + + notnull = makeNode(Constraint); + notnull->contype = CONSTR_NOTNULL; + notnull->conname = NULL; + notnull->is_no_inherit = false; + notnull->deferrable = false; + notnull->initdeferred = false; + notnull->location = -1; + notnull->colname = pstrdup(NameStr(attForm->attname)); + notnull->skip_validation = false; + notnull->initially_valid = true; + + cxt->nnconstraints = lappend(cxt->nnconstraints, notnull); + } + } + /* * We may copy extended statistics if requested, since the representation * of CreateStatsStmt doesn't depend on column numbers. @@ -1201,6 +1313,8 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) TupleConstr *constr; AttrMap *attmap; char *comment; + bool at_pushed = false; + ListCell *lc; /* * Open the relation referenced by the LIKE clause. We should still have @@ -1380,6 +1494,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 @@ -1394,6 +1522,8 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) atcmd->objtype = OBJECT_TABLE; atcmd->missing_ok = false; result = lcons(atcmd, result); + + at_pushed = true; } /* @@ -1421,6 +1551,39 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause) attmap, NULL); + /* + * The PK columns might not yet non-nullable, so make sure they + * become so. + */ + if (index_stmt->primary) + { + foreach(lc, index_stmt->indexParams) + { + IndexElem *col = lfirst_node(IndexElem, lc); + AlterTableCmd *notnullcmd = makeNode(AlterTableCmd); + + notnullcmd->subtype = AT_SetAttNotNull; + notnullcmd->name = pstrdup(col->name); + /* Luckily we can still add more AT-subcmds here */ + atsubcmds = lappend(atsubcmds, notnullcmd); + } + + /* + * If we had already put the AlterTableStmt into the output + * list, we don't need to do so again; otherwise do it. + */ + if (!at_pushed) + { + AlterTableStmt *atcmd = makeNode(AlterTableStmt); + + atcmd->relation = copyObject(heapRel); + atcmd->cmds = atsubcmds; + atcmd->objtype = OBJECT_TABLE; + atcmd->missing_ok = false; + result = lcons(atcmd, result); + } + } + /* Copy comment on index, if requested */ if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS) { @@ -2057,10 +2220,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) { @@ -2134,9 +2299,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); } @@ -2144,12 +2307,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) @@ -2415,7 +2576,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; @@ -2436,13 +2596,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) @@ -2485,14 +2646,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; } } @@ -2546,15 +2699,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); } @@ -3326,6 +3475,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; @@ -3569,8 +3719,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)) { @@ -3593,19 +3743,26 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, } cxt.alist = NIL; - /* Append any CHECK or FK constraints to the commands list */ + /* Append any CHECK, NOT NULL or FK constraints to the commands list */ foreach(l, cxt.ckconstraints) { 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); } 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); } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index d3a973d86b..224fd37fcf 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -2490,6 +2490,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/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 8a08463c2b..b5a99b4edc 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -4788,19 +4788,28 @@ RelationGetIndexList(Relation relation) result = lappend_oid(result, index->indexrelid); /* - * Invalid, non-unique, non-immediate or predicate indexes aren't - * interesting for either oid indexes or replication identity indexes, - * so don't check them. + * Non-unique, non-immediate or predicate indexes aren't interesting + * for either oid indexes or replication identity indexes, so don't + * check them. */ - if (!index->indisvalid || !index->indisunique || + if (!index->indisunique || !index->indimmediate || !heap_attisnull(htup, Anum_pg_index_indpred, NULL)) continue; - /* remember primary key index if any */ - if (index->indisprimary) + /* + * Remember primary key index, if any. We do this only if the index + * is valid; but if the table is partitioned, then we do it even if + * it's invalid. XXX does this cause other problems? + */ + if (index->indisprimary && + (index->indisvalid || + relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)) pkeyIndex = index->indexrelid; + if (!index->indisvalid) + continue; + /* remember explicitly chosen replica index */ if (index->indisreplident) candidateIndex = index->indexrelid; diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 5d988986ed..8b0c1e7b53 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, @@ -554,7 +556,8 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables) { AttrDefInfo *parentDef = parent->attrdefs[inhAttrInd]; - foundNotNull |= parent->notnull[inhAttrInd]; + foundNotNull |= (parent->notnull_constrs[inhAttrInd] != NULL && + !parent->notnull_noinh[inhAttrInd]); foundDefault |= (parentDef != NULL && strcmp(parentDef->adef_expr, "NULL") != 0 && !parent->attgenerated[inhAttrInd]); @@ -572,8 +575,9 @@ flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables) } } - /* Remember if we found inherited NOT NULL */ - tbinfo->inhNotNull[j] = foundNotNull; + /* In versions < 17, remember if we found inherited NOT NULL */ + if (fout->remoteVersion < 170000) + tbinfo->notnull_inh[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 39ebcfec32..71627ca2a7 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -602,6 +602,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 @@ -3513,6 +3514,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 5dab1ba9ea..a55d396f34 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -4864,7 +4864,7 @@ append_depends_on_extension(Archive *fout, i_extname = PQfnumber(res, "extname"); for (i = 0; i < ntups; i++) { - appendPQExpBuffer(create, "ALTER %s %s DEPENDS ON EXTENSION %s;\n", + appendPQExpBuffer(create, "\nALTER %s %s DEPENDS ON EXTENSION %s;", keyword, nm, fmtId(PQgetvalue(res, i, i_extname))); } @@ -8373,7 +8373,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) int i_attlen; int i_attalign; int i_attislocal; - int i_attnotnull; + int i_notnull_name; + int i_notnull_noinherit; + int i_notnull_is_pk; + int i_notnull_inh; int i_attoptions; int i_attcollation; int i_attcompression; @@ -8383,13 +8386,13 @@ 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, '{'); @@ -8436,7 +8439,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" @@ -8453,6 +8455,32 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "ORDER BY option_name" "), E',\n ') AS attfdwoptions,\n"); + /* + * Find out any NOT NULL markings for each column. In 17 and up we have + * to read pg_constraint, and keep track whether it's NO INHERIT; in older + * versions we rely on pg_attribute.attnotnull. + * + * We also track whether the constraint was defined directly in this table + * or via an ancestor, for binary upgrade. Lastly, we need to know if the + * PK for the table involves each column; for columns that are there we + * need a NOT NULL marking even if there's no explicit constraint, to + * avoid the table having to be scanned for NULLs after the data is loaded + * when the PK is created, later in the dump; for this case we add + * throwaway constraints that are dropped once the PK is created. + */ + if (fout->remoteVersion >= 170000) + appendPQExpBufferStr(q, + "co.conname AS notnull_name,\n" + "co.connoinherit AS notnull_noinherit,\n" + "copk.conname IS NOT NULL as notnull_is_pk,\n" + "coalesce(NOT co.conislocal, true) AS notnull_inh,\n"); + else + appendPQExpBufferStr(q, + "CASE WHEN a.attnotnull THEN '' ELSE NULL END AS notnull_name,\n" + "false AS notnull_noinherit,\n" + "copk.conname IS NOT NULL AS notnull_is_pk,\n" + "NOT a.attislocal AS notnull_inh,\n"); + if (fout->remoteVersion >= 140000) appendPQExpBufferStr(q, "a.attcompression AS attcompression,\n"); @@ -8487,11 +8515,29 @@ 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); + /* + * In versions 16 and up, we need pg_constraint for explicit NOT NULL + * entries. Also, we need to know if the NOT NULL for each column is + * backing a primary key. + */ + if (fout->remoteVersion >= 170000) + 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"); + + appendPQExpBufferStr(q, + "LEFT JOIN pg_catalog.pg_constraint copk ON " + "(copk.conrelid = src.tbloid\n" + " AND copk.contype = 'p' AND " + "copk.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); ntups = PQntuples(res); @@ -8509,7 +8555,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_attlen = PQfnumber(res, "attlen"); i_attalign = PQfnumber(res, "attalign"); i_attislocal = PQfnumber(res, "attislocal"); - i_attnotnull = PQfnumber(res, "attnotnull"); + i_notnull_name = PQfnumber(res, "notnull_name"); + i_notnull_noinherit = PQfnumber(res, "notnull_noinherit"); + i_notnull_is_pk = PQfnumber(res, "notnull_is_pk"); + i_notnull_inh = PQfnumber(res, "notnull_inh"); i_attoptions = PQfnumber(res, "attoptions"); i_attcollation = PQfnumber(res, "attcollation"); i_attcompression = PQfnumber(res, "attcompression"); @@ -8532,6 +8581,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) TableInfo *tbinfo = NULL; int numatts; bool hasdefaults; + int notnullcount; /* Count rows for this table */ for (numatts = 1; numatts < ntups - r; numatts++) @@ -8556,6 +8606,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) pg_fatal("unexpected column data for table \"%s\"", tbinfo->dobj.name); + notnullcount = 0; + /* Save data for this table */ tbinfo->numatts = numatts; tbinfo->attnames = (char **) pg_malloc(numatts * sizeof(char *)); @@ -8574,13 +8626,19 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attcompression = (char *) pg_malloc(numatts * sizeof(char)); 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->notnull_constrs = (char **) pg_malloc(numatts * sizeof(char *)); + tbinfo->notnull_noinh = (bool *) pg_malloc(numatts * sizeof(bool)); + tbinfo->notnull_throwaway = (bool *) pg_malloc(numatts * sizeof(bool)); + tbinfo->notnull_inh = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *)); hasdefaults = false; for (int j = 0; j < numatts; j++, r++) { + bool use_named_notnull = false; + bool use_unnamed_notnull = false; + bool use_throwaway_notnull = false; + if (j + 1 != atoi(PQgetvalue(res, r, i_attnum))) pg_fatal("invalid column numbering in table \"%s\"", tbinfo->dobj.name); @@ -8596,7 +8654,129 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attlen[j] = atoi(PQgetvalue(res, r, i_attlen)); 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'); + + /* + * NOT NULL constraints require a jumping through a few hoops. + * First, if the user has specified a constraint name that's not + * the system-assigned default name, then we need to preserve + * that. But if they haven't, then we don't want to use the + * verbose syntax in the dump output. (Also, in versions prior to + * 17, there was no constraint name at all.) + * + * (XXX Comparing the name this way to a supposed default name is + * a bit of a hack, but it beats having to store a boolean flag in + * pg_constraint just for this, or having to compute the knowledge + * at pg_dump time from the server.) + * + * We also need to know if a column is part of the primary key. In + * that case, we want to mark the column as NOT NULL at table + * creation time, so that the table doesn't have to be scanned to + * check for nulls when the PK is created afterwards; this is + * especially critical during pg_upgrade (where the data would not + * be scanned at all otherwise.) If the column is part of the PK + * and does not have any other NOT NULL constraint, then we + * fabricate a throwaway constraint name that we later use to + * remove the constraint after the PK has been created. + * + * For inheritance child tables, we don't want to print NOT NULL + * when the constraint was defined at the parent level instead of + * locally. + */ + + /* + * We use notnull_inh to suppress unwanted NOT NULL constraints in + * inheritance children, when said constraints come from the + * parent(s). + */ + tbinfo->notnull_inh[j] = PQgetvalue(res, r, i_notnull_inh)[0] == 't'; + + if (fout->remoteVersion < 170000) + { + if (!PQgetisnull(res, r, i_notnull_name) && + dopt->binary_upgrade && + !tbinfo->ispartition && + tbinfo->notnull_inh[j]) + { + use_named_notnull = true; + /* XXX should match ChooseConstraintName better */ + tbinfo->notnull_constrs[j] = + psprintf("%s_%s_not_null", tbinfo->dobj.name, + tbinfo->attnames[j]); + } + else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't') + use_throwaway_notnull = true; + else if (!PQgetisnull(res, r, i_notnull_name)) + use_unnamed_notnull = true; + } + else + { + if (!PQgetisnull(res, r, i_notnull_name)) + { + /* + * In binary upgrade of inheritance child tables, must + * have a constraint name that we can UPDATE later. + */ + if (dopt->binary_upgrade && + !tbinfo->ispartition && + tbinfo->notnull_inh[j]) + { + use_named_notnull = true; + tbinfo->notnull_constrs[j] = + pstrdup(PQgetvalue(res, r, i_notnull_name)); + + } + else + { + char *default_name; + + /* XXX should match ChooseConstraintName better */ + default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name, + tbinfo->attnames[j]); + if (strcmp(default_name, + PQgetvalue(res, r, i_notnull_name)) == 0) + use_unnamed_notnull = true; + else + { + use_named_notnull = true; + tbinfo->notnull_constrs[j] = + pstrdup(PQgetvalue(res, r, i_notnull_name)); + } + } + } + else if (PQgetvalue(res, r, i_notnull_is_pk)[0] == 't') + use_throwaway_notnull = true; + } + + if (use_unnamed_notnull) + { + tbinfo->notnull_constrs[j] = ""; + tbinfo->notnull_throwaway[j] = false; + } + else if (use_named_notnull) + { + /* The name itself has already been determined */ + tbinfo->notnull_throwaway[j] = false; + } + else if (use_throwaway_notnull) + { + tbinfo->notnull_constrs[j] = + psprintf("pgdump_throwaway_notnull_%d", notnullcount++); + tbinfo->notnull_throwaway[j] = true; + tbinfo->notnull_inh[j] = false; + } + else + { + tbinfo->notnull_constrs[j] = NULL; + tbinfo->notnull_throwaway[j] = false; + } + + /* + * Throwaway constraints must always be NO INHERIT; otherwise do + * what the catalog says. + */ + tbinfo->notnull_noinh[j] = use_throwaway_notnull || + PQgetvalue(res, r, i_notnull_noinherit)[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)); @@ -8605,8 +8785,6 @@ 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) @@ -15561,13 +15739,14 @@ 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->ispartition || dopt->binary_upgrade)); + print_notnull = + (tbinfo->notnull_constrs[j] != NULL && + (!tbinfo->notnull_inh[j] || tbinfo->ispartition || + dopt->binary_upgrade)); /* * Skip column if fully defined by reloftype, except in @@ -15625,7 +15804,16 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) if (print_notnull) - appendPQExpBufferStr(q, " NOT NULL"); + { + if (tbinfo->notnull_constrs[j][0] == '\0') + appendPQExpBufferStr(q, " NOT NULL"); + else + appendPQExpBuffer(q, " CONSTRAINT %s NOT NULL", + fmtId(tbinfo->notnull_constrs[j])); + + if (tbinfo->notnull_noinh[j]) + appendPQExpBufferStr(q, " NO INHERIT"); + } /* Add collation if not default for the type */ if (OidIsValid(tbinfo->attcollation[j])) @@ -15838,6 +16026,21 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) appendPQExpBufferStr(q, "\n AND attrelid = "); appendStringLiteralAH(q, qualrelname, fout); appendPQExpBufferStr(q, "::pg_catalog.regclass;\n"); + + if (tbinfo->notnull_constrs[j] != NULL && + !tbinfo->notnull_throwaway[j] && + tbinfo->notnull_inh[j] && + !tbinfo->ispartition) + { + appendPQExpBufferStr(q, "UPDATE pg_catalog.pg_constraint\n" + "SET conislocal = false\n" + "WHERE contype = 'n' AND conrelid = "); + appendStringLiteralAH(q, qualrelname, fout); + appendPQExpBufferStr(q, "::pg_catalog.regclass AND\n" + "conname = "); + appendStringLiteralAH(q, tbinfo->notnull_constrs[j], fout); + appendPQExpBufferStr(q, ";\n"); + } } } @@ -15959,11 +16162,22 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) * we have to mark it separately. */ if (!shouldPrintColumn(dopt, tbinfo, j) && - tbinfo->notnull[j] && !tbinfo->inhNotNull[j]) - appendPQExpBuffer(q, - "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", - foreign, qualrelname, - fmtId(tbinfo->attnames[j])); + tbinfo->notnull_constrs[j] != NULL && + (!tbinfo->notnull_inh[j] && !tbinfo->ispartition && !dopt->binary_upgrade)) + { + /* pre-v16 NOT NULL constraints don't have names */ + if (tbinfo->notnull_constrs[j][0] == '\0') + appendPQExpBuffer(q, + "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", + foreign, qualrelname, + fmtId(tbinfo->attnames[j])); + else + appendPQExpBuffer(q, + "ALTER %sTABLE ONLY %s ADD CONSTRAINT %s NOT NULL %s;\n", + foreign, qualrelname, + tbinfo->notnull_constrs[j], + fmtId(tbinfo->attnames[j])); + } /* * Dump per-column statistics information. We only issue an ALTER @@ -16704,6 +16918,20 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo) * similar code in dumpIndex! */ + /* Drop any NOT NULL constraints that were added to support the PK */ + if (coninfo->contype == 'p') + { + for (int i = 0; i < tbinfo->numatts; i++) + { + if (tbinfo->notnull_throwaway[i]) + { + appendPQExpBuffer(q, "\nALTER TABLE ONLY %s DROP CONSTRAINT %s;", + fmtQualifiedDumpable(tbinfo), + tbinfo->notnull_constrs[i]); + } + } + } + /* If the index is clustered, we need to record that. */ if (indxinfo->indisclustered) { diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index bc8f2ec36d..9036b13f6a 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -345,8 +345,13 @@ typedef struct _tableInfo char *attcompression; /* per-attribute compression method */ 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 */ + char **notnull_constrs; /* NOT NULL constraint names. If null, + * there isn't one on this column. If + * empty string, unnamed constraint + * (pre-v17) */ + bool *notnull_noinh; /* NOT NULL is NO INHERIT */ + bool *notnull_throwaway; /* drop the NOT NULL constraint later */ + bool *notnull_inh; /* true if NOT NULL has no 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 d42243bf71..e3ca191dbf 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -3194,7 +3194,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 CONSTRAINT \E[a-z0-9_]*\Q NOT NULL NO INHERIT\E \n\); /xm, like => @@ -3292,8 +3292,8 @@ my %tests = ( FOR VALUES FROM (\'2006-02-01\') TO (\'2006-03-01\');', regexp => qr/^ \QCREATE TABLE dump_test_second_schema.measurement_y2006m2 (\E\n - \s+\Qcity_id integer DEFAULT nextval('dump_test.measurement_city_id_seq'::regclass) NOT NULL,\E\n - \s+\Qlogdate date NOT NULL,\E\n + \s+\Qcity_id integer DEFAULT nextval('dump_test.measurement_city_id_seq'::regclass) CONSTRAINT measurement_city_id_not_null NOT NULL,\E\n + \s+\Qlogdate date CONSTRAINT measurement_logdate_not_null NOT NULL,\E\n \s+\Qpeaktemp integer,\E\n \s+\Qunitsales integer DEFAULT 0,\E\n \s+\QCONSTRAINT measurement_peaktemp_check CHECK ((peaktemp >= '-460'::integer)),\E\n @@ -3588,7 +3588,7 @@ my %tests = ( );', regexp => qr/^ \QCREATE TABLE dump_test.test_table_generated (\E\n - \s+\Qcol1 integer NOT NULL,\E\n + \s+\Qcol1 integer CONSTRAINT \E[a-z0-9_]*\Q NOT NULL NO INHERIT,\E\n \s+\Qcol2 integer GENERATED ALWAYS AS ((col1 * 2)) STORED\E\n \); /xms, @@ -3702,7 +3702,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..64f5374c17 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -34,10 +34,11 @@ typedef struct RawColumnDefault typedef struct CookedConstraint { - ConstrType contype; /* CONSTR_DEFAULT or CONSTR_CHECK */ + ConstrType contype; /* CONSTR_DEFAULT, CONSTR_CHECK, + * CONSTR_NOTNULL */ 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 +114,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 16b6126669..b56ccd4d38 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 efb5c3e098..7189c2a769 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2178,8 +2178,8 @@ 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 */ AT_SetOptions, /* alter column set ( options ) */ AT_ResetOptions, /* alter column reset ( options ) */ @@ -2462,10 +2462,10 @@ 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 nnconstraints are NIL. After + * parse analysis, tableElts contains just ColumnDefs, nnconstraints contains + * Constraint nodes of CONSTR_NOTNULL type from various sources, and + * constraints contains just CONSTR_CHECK Constraint nodes. * ---------------------- */ @@ -2480,6 +2480,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 +2569,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..ecde9d7422 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 column a of table part +NOTICE: subcommand: type SET ATTNOTNULL desc column a of table part1 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..75b62aff4d 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 column name of table employees 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 @@ -136,7 +146,13 @@ 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 SET ATTNOTNULL desc column id of table like_fkey_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 +160,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 82f937fca4..0302f79bb7 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -129,12 +129,12 @@ 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; - case AT_CheckNotNull: - strtype = "CHECK NOT NULL"; - break; case AT_SetStatistics: strtype = "SET STATS"; break; diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 05351cb1a4..06cf1d4bb2 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1118,10 +1118,30 @@ ERROR: relation "non_existent" does not exist -- test checking for null values and primary key create table atacc1 (test int not null); alter table atacc1 add constraint "atacc1_pkey" primary key (test); +\d atacc1 + Table "public.atacc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + test | integer | | not null | +Indexes: + "atacc1_pkey" PRIMARY KEY, btree (test) + alter table atacc1 alter column test drop not null; -ERROR: column "test" is in a primary key +\d atacc1 + Table "public.atacc1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + test | integer | | not null | +Indexes: + "atacc1_pkey" PRIMARY KEY, btree (test) + 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 @@ -1194,20 +1214,6 @@ alter table only parent alter a set not null; ERROR: column "a" of relation "parent" contains null values 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 +3840,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, @@ -4356,7 +4384,6 @@ ERROR: cannot alter inherited column "b" -- 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. 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 542c2e098c..a666d89ef5 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..e92d99d701 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,101 @@ 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; +-- nope +CREATE TABLE notnull_tbl2 (a INTEGER CONSTRAINT blah NOT NULL, b INTEGER CONSTRAINT blah NOT NULL); +ERROR: constraint "blah" for relation "notnull_tbl2" already exists +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 1c3ef2b05a..6229dd5c70 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; - conislocal | coninhcount -------------+------------- - f | 1 - f | 1 -(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 + check_b | f | 1 + part_b_b_not_null | 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 DESC, conname; + 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 12e523c737..af2a878dd6 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 3e5645c2ab..c60e83ec37 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..21dfe9925d 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1847,6 +1847,414 @@ 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) + order by 1, 2; + 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) + order by 1, 2; + 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) + order by 1, 2; + 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 inh_child1() inherits(inh_parent); +create table inh_child2() inherits(inh_parent); +create table inh_grandchld() inherits(inh_child1, inh_child2); +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, 'inh_child1'::regclass, 'inh_child2'::regclass, 'inh_grandchld'::regclass) + order by 2, conrelid::regclass::text; + conrelid | conname | contype | coninhcount | conislocal +---------------+------------------------+---------+-------------+------------ + inh_child1 | inh_parent_f1_not_null | n | 1 | f + inh_child2 | inh_parent_f1_not_null | n | 1 | f + inh_grandchld | inh_parent_f1_not_null | n | 2 | f + inh_parent | inh_parent_f1_not_null | n | 0 | t +(4 rows) + +drop table inh_parent cascade; +NOTICE: drop cascades to 3 other objects +DETAIL: drop cascades to table inh_child1 +drop cascades to table inh_child2 +drop cascades to table inh_grandchld +-- 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 inh_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, 'inh_child'::regclass) + order by 2, conrelid::regclass::text; + conrelid | conname | contype | coninhcount | conislocal +-----------+-----------------------+---------+-------------+------------ + inh_child | inh_child_f1_not_null | n | 0 | t + inh_child | inh_child_f2_not_null | n | 0 | t +(2 rows) + +-- also drops inh_child table +drop table inh_parent_1 cascade; +NOTICE: drop cascades to table inh_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 inh_multiparent() 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" +-- constraint on f1 should have three parents +select conrelid::regclass, contype, conname, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid::regclass in ('inh_p1', 'inh_p2', 'inh_p3', 'inh_p4', + 'inh_multiparent') + order by conrelid::regclass::text, conname; + conrelid | contype | conname | attname | coninhcount | conislocal +-----------------+---------+--------------------+---------+-------------+------------ + inh_multiparent | n | inh_p1_f1_not_null | f1 | 3 | f + inh_multiparent | n | inh_p4_f3_not_null | f3 | 1 | f + inh_p1 | n | inh_p1_f1_not_null | f1 | 0 | t + inh_p2 | n | inh_p2_f1_not_null | f1 | 0 | t + inh_p4 | n | inh_p4_f1_not_null | f1 | 0 | t + inh_p4 | n | inh_p4_f3_not_null | f3 | 0 | t +(6 rows) + +create table inh_multiparent2 (a int not null, f1 int) inherits(inh_p3, inh_multiparent); +NOTICE: merging multiple inherited definitions of column "f2" +NOTICE: merging column "f1" with inherited definition +select conrelid::regclass, contype, conname, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid::regclass in ('inh_p3', 'inh_multiparent', 'inh_multiparent2') + order by conrelid::regclass::text, conname; + conrelid | contype | conname | attname | coninhcount | conislocal +------------------+---------+-----------------------------+---------+-------------+------------ + inh_multiparent | n | inh_p1_f1_not_null | f1 | 3 | f + inh_multiparent | n | inh_p4_f3_not_null | f3 | 1 | f + inh_multiparent2 | n | inh_multiparent2_a_not_null | a | 0 | t + inh_multiparent2 | n | inh_p1_f1_not_null | f1 | 1 | f + inh_multiparent2 | n | inh_p4_f3_not_null | f3 | 1 | f +(5 rows) + +drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table inh_multiparent +drop cascades to table inh_multiparent2 +-- -- 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..0a62b28823 100644 --- a/src/test/regress/expected/replica_identity.out +++ b/src/test/regress/expected/replica_identity.out @@ -227,6 +227,9 @@ Indexes: -- used as replica identity. ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; ERROR: column "id" is in index used as replica identity +-- but it's OK when the identity is FULL +ALTER TABLE test_replica_identity3 REPLICA IDENTITY FULL; +ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; -- -- Test that replica identity can be set on an index that's not yet valid. -- (This matches the way pg_dump will try to dump a partitioned table.) @@ -263,8 +266,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 cf46fa3359..4df9d8503b 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..5ac6147e92 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -850,9 +850,11 @@ alter table non_existent alter column bar drop not null; -- test checking for null values and primary key create table atacc1 (test int not null); alter table atacc1 add constraint "atacc1_pkey" primary key (test); +\d atacc1 alter table atacc1 alter column test drop not null; +\d atacc1 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 +919,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 +2336,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..dbeab30e2d 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,41 @@ 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; + +-- nope +CREATE TABLE notnull_tbl2 (a INTEGER CONSTRAINT blah NOT NULL, b INTEGER CONSTRAINT blah NOT NULL); + +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..2d05987141 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 conname, 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 DESC, conname; -- 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 d6e5a06d95..f91f648a17 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..e940ae2997 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -679,6 +679,217 @@ 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) + order by 1, 2; +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) + order by 1, 2; +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) + order by 1, 2; +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 inh_child1() inherits(inh_parent); +create table inh_child2() inherits(inh_parent); +create table inh_grandchld() inherits(inh_child1, inh_child2); + +-- show constraint info +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, 'inh_grandchld'::regclass) + order by 2, conrelid::regclass::text; + +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 inh_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, 'inh_child'::regclass) + order by 2, conrelid::regclass::text; + +-- also drops inh_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 inh_multiparent() inherits(inh_p1, inh_p2, inh_p3, inh_p4); + +-- constraint on f1 should have three parents +select conrelid::regclass, contype, conname, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid::regclass in ('inh_p1', 'inh_p2', 'inh_p3', 'inh_p4', + 'inh_multiparent') + order by conrelid::regclass::text, conname; + +create table inh_multiparent2 (a int not null, f1 int) inherits(inh_p3, inh_multiparent); +select conrelid::regclass, contype, conname, + (select attname from pg_attribute where attrelid = conrelid and attnum = conkey[1]), + coninhcount, conislocal + from pg_constraint where contype = 'n' and + conrelid::regclass in ('inh_p3', 'inh_multiparent', 'inh_multiparent2') + order by conrelid::regclass::text, conname; + +drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade; + -- -- Check use of temporary tables with inheritance trees -- diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sql index 14620b7713..dd43650586 100644 --- a/src/test/regress/sql/replica_identity.sql +++ b/src/test/regress/sql/replica_identity.sql @@ -97,6 +97,9 @@ ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint; -- ALTER TABLE DROP NOT NULL is not allowed for columns part of an index -- used as replica identity. ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; +-- but it's OK when the identity is FULL +ALTER TABLE test_replica_identity3 REPLICA IDENTITY FULL; +ALTER TABLE test_replica_identity3 ALTER COLUMN id DROP NOT NULL; -- -- Test that replica identity can be set on an index that's not yet valid. @@ -117,8 +120,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