From 778fd05ba7fde5062e64addc98eda5505ef475ca Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 23 Nov 2023 07:35:32 +0100 Subject: [PATCH v1 2/2] Catalog domain not-null constraints This applies the explicit catalog representation of not-null constraints introduced by b0e96f3119 for table constraints also to domain not-null constraints. TODO: catversion --- src/backend/catalog/information_schema.sql | 2 +- src/backend/catalog/pg_constraint.c | 40 +++ src/backend/commands/typecmds.c | 313 +++++++++++++++------ src/backend/utils/cache/typcache.c | 2 +- src/bin/pg_dump/pg_dump.c | 2 +- src/include/catalog/pg_constraint.h | 1 + src/test/regress/expected/domain.out | 49 +++- src/test/regress/sql/domain.sql | 26 ++ 8 files changed, 331 insertions(+), 104 deletions(-) diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index 10b34c3c5b..95a1b34560 100644 --- a/src/backend/catalog/information_schema.sql +++ b/src/backend/catalog/information_schema.sql @@ -448,7 +448,7 @@ CREATE VIEW check_constraints AS SELECT current_database()::information_schema.sql_identifier AS constraint_catalog, rs.nspname::information_schema.sql_identifier AS constraint_schema, con.conname::information_schema.sql_identifier AS constraint_name, - pg_catalog.format('%s IS NOT NULL', at.attname)::information_schema.character_data AS check_clause + pg_catalog.format('%s IS NOT NULL', coalesce(at.attname, 'VALUE'))::information_schema.character_data AS check_clause FROM pg_constraint con LEFT JOIN pg_namespace rs ON rs.oid = con.connamespace LEFT JOIN pg_class c ON c.oid = con.conrelid diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index e9d4d6006e..4f1a5e1c84 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -629,6 +629,46 @@ findNotNullConstraint(Oid relid, const char *colname) return findNotNullConstraintAttnum(relid, attnum); } +HeapTuple +findDomainNotNullConstraint(Oid typid) +{ + Relation pg_constraint; + HeapTuple conTup, + retval = NULL; + SysScanDesc scan; + ScanKeyData key; + + pg_constraint = table_open(ConstraintRelationId, AccessShareLock); + ScanKeyInit(&key, + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(typid)); + 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); + + /* + * We're looking for a NOTNULL constraint that's marked validated. + */ + if (con->contype != CONSTRAINT_NOTNULL) + continue; + if (!con->convalidated) + continue; + + /* Found it */ + retval = heap_copytuple(conTup); + break; + } + + systable_endscan(scan); + table_close(pg_constraint, AccessShareLock); + + return retval; +} + /* * Given a pg_constraint tuple for a not-null constraint, return the column * number it is for. diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index aaf9728697..57389be9ed 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -126,15 +126,19 @@ static Oid findTypeSubscriptingFunction(List *procname, Oid typeOid); static Oid findRangeSubOpclass(List *opcname, Oid subtype); static Oid findRangeCanonicalFunction(List *procname, Oid typeOid); static Oid findRangeSubtypeDiffFunction(List *procname, Oid subtype); -static void validateDomainConstraint(Oid domainoid, char *ccbin); +static void validateDomainCheckConstraint(Oid domainoid, char *ccbin); +static void validateDomainNotNullConstraint(Oid domainoid); static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode); static void checkEnumOwner(HeapTuple tup); -static char *domainAddConstraint(Oid domainOid, Oid domainNamespace, - Oid baseTypeOid, - int typMod, Constraint *constr, - const char *domainName, ObjectAddress *constrAddr); +static char *domainAddCheckConstraint(Oid domainOid, Oid domainNamespace, + Oid baseTypeOid, + int typMod, Constraint *constr, + const char *domainName, ObjectAddress *constrAddr); static Node *replace_domain_constraint_value(ParseState *pstate, ColumnRef *cref); +static void domainAddNotNullConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, + int typMod, Constraint *constr, + const char *domainName, ObjectAddress *constrAddr); static void AlterTypeRecurse(Oid typeOid, bool isImplicitArray, HeapTuple tup, Relation catalog, AlterTypeRecurseParams *atparams); @@ -1105,9 +1109,15 @@ DefineDomain(CreateDomainStmt *stmt) switch (constr->contype) { case CONSTR_CHECK: - domainAddConstraint(address.objectId, domainNamespace, - basetypeoid, basetypeMod, - constr, domainName, NULL); + domainAddCheckConstraint(address.objectId, domainNamespace, + basetypeoid, basetypeMod, + constr, domainName, NULL); + break; + + case CONSTR_NOTNULL: + domainAddNotNullConstraint(address.objectId, domainNamespace, + basetypeoid, basetypeMod, + constr, domainName, NULL); break; /* Other constraint types were fully processed above */ @@ -2723,66 +2733,32 @@ AlterDomainNotNull(List *names, bool notNull) return address; } - /* Adding a NOT NULL constraint requires checking existing columns */ if (notNull) { - List *rels; - ListCell *rt; + Constraint *constr; - /* Fetch relation list with attributes based on this domain */ - /* ShareLock is sufficient to prevent concurrent data changes */ + constr = makeNode(Constraint); + constr->contype = CONSTR_NOTNULL; + constr->initially_valid = true; + constr->location = -1; - rels = get_rels_with_domain(domainoid, ShareLock); - - foreach(rt, rels) - { - RelToCheck *rtc = (RelToCheck *) lfirst(rt); - Relation testrel = rtc->rel; - TupleDesc tupdesc = RelationGetDescr(testrel); - TupleTableSlot *slot; - TableScanDesc scan; - Snapshot snapshot; - - /* Scan all tuples in this relation */ - snapshot = RegisterSnapshot(GetLatestSnapshot()); - scan = table_beginscan(testrel, snapshot, 0, NULL); - slot = table_slot_create(testrel, NULL); - while (table_scan_getnextslot(scan, ForwardScanDirection, slot)) - { - int i; + domainAddNotNullConstraint(domainoid, typTup->typnamespace, + typTup->typbasetype, typTup->typtypmod, + constr, NameStr(typTup->typname), NULL); - /* Test attributes that are of the domain */ - for (i = 0; i < rtc->natts; i++) - { - int attnum = rtc->atts[i]; - Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + validateDomainNotNullConstraint(domainoid); + } + else + { + HeapTuple conTup; + ObjectAddress conobj; - if (slot_attisnull(slot, attnum)) - { - /* - * In principle the auxiliary information for this - * error should be errdatatype(), but errtablecol() - * seems considerably more useful in practice. Since - * this code only executes in an ALTER DOMAIN command, - * the client should already know which domain is in - * question. - */ - ereport(ERROR, - (errcode(ERRCODE_NOT_NULL_VIOLATION), - errmsg("column \"%s\" of table \"%s\" contains null values", - NameStr(attr->attname), - RelationGetRelationName(testrel)), - errtablecol(testrel, attnum))); - } - } - } - ExecDropSingleTupleTableSlot(slot); - table_endscan(scan); - UnregisterSnapshot(snapshot); + conTup = findDomainNotNullConstraint(domainoid); + if (conTup == NULL) + elog(ERROR, "could not find not-null constraint on domain \"%s\"", NameStr(typTup->typname)); - /* Close each rel after processing, but keep lock */ - table_close(testrel, NoLock); - } + ObjectAddressSet(conobj, ConstraintRelationId, ((Form_pg_constraint) GETSTRUCT(conTup))->oid); + performDeletion(&conobj, DROP_RESTRICT, 0); } /* @@ -2863,10 +2839,17 @@ AlterDomainDropConstraint(List *names, const char *constrName, /* There can be at most one matching row */ if ((contup = systable_getnext(conscan)) != NULL) { + Form_pg_constraint construct = (Form_pg_constraint) GETSTRUCT(contup); ObjectAddress conobj; + if (construct->contype == CONSTRAINT_NOTNULL) + { + ((Form_pg_type) GETSTRUCT(tup))->typnotnull = false; + CatalogTupleUpdate(rel, &tup->t_self, tup); + } + conobj.classId = ConstraintRelationId; - conobj.objectId = ((Form_pg_constraint) GETSTRUCT(contup))->oid; + conobj.objectId = construct->oid; conobj.objectSubId = 0; performDeletion(&conobj, behavior, 0); @@ -2947,6 +2930,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, switch (constr->contype) { case CONSTR_CHECK: + case CONSTR_NOTNULL: /* processed below */ break; @@ -2989,29 +2973,44 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, break; } - /* - * Since all other constraint types throw errors, this must be a check - * constraint. First, process the constraint expression and add an entry - * to pg_constraint. - */ + if (constr->contype == CONSTR_CHECK) + { + /* + * First, process the constraint expression and add an entry + * to pg_constraint. + */ - ccbin = domainAddConstraint(domainoid, typTup->typnamespace, - typTup->typbasetype, typTup->typtypmod, - constr, NameStr(typTup->typname), constrAddr); + ccbin = domainAddCheckConstraint(domainoid, typTup->typnamespace, + typTup->typbasetype, typTup->typtypmod, + constr, NameStr(typTup->typname), constrAddr); - /* - * If requested to validate the constraint, test all values stored in the - * attributes based on the domain the constraint is being added to. - */ - if (!constr->skip_validation) - validateDomainConstraint(domainoid, ccbin); - /* - * We must send out an sinval message for the domain, to ensure that any - * dependent plans get rebuilt. Since this command doesn't change the - * domain's pg_type row, that won't happen automatically; do it manually. - */ - CacheInvalidateHeapTuple(typrel, tup, NULL); + /* + * If requested to validate the constraint, test all values stored in the + * attributes based on the domain the constraint is being added to. + */ + if (!constr->skip_validation) + validateDomainCheckConstraint(domainoid, ccbin); + + /* + * We must send out an sinval message for the domain, to ensure that any + * dependent plans get rebuilt. Since this command doesn't change the + * domain's pg_type row, that won't happen automatically; do it manually. + */ + CacheInvalidateHeapTuple(typrel, tup, NULL); + } + else if (constr->contype == CONSTR_NOTNULL) + { + domainAddNotNullConstraint(domainoid, typTup->typnamespace, + typTup->typbasetype, typTup->typtypmod, + constr, NameStr(typTup->typname), constrAddr); + + if (!constr->skip_validation) + validateDomainNotNullConstraint(domainoid); + + typTup->typnotnull = true; + CatalogTupleUpdate(typrel, &tup->t_self, tup); + } ObjectAddressSet(address, TypeRelationId, domainoid); @@ -3096,7 +3095,7 @@ AlterDomainValidateConstraint(List *names, const char *constrName) val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin); conbin = TextDatumGetCString(val); - validateDomainConstraint(domainoid, conbin); + validateDomainCheckConstraint(domainoid, conbin); /* * Now update the catalog, while we have the door open. @@ -3123,7 +3122,69 @@ AlterDomainValidateConstraint(List *names, const char *constrName) } static void -validateDomainConstraint(Oid domainoid, char *ccbin) +validateDomainNotNullConstraint(Oid domainoid) +{ + List *rels; + ListCell *rt; + + /* Fetch relation list with attributes based on this domain */ + /* ShareLock is sufficient to prevent concurrent data changes */ + + rels = get_rels_with_domain(domainoid, ShareLock); + + foreach(rt, rels) + { + RelToCheck *rtc = (RelToCheck *) lfirst(rt); + Relation testrel = rtc->rel; + TupleDesc tupdesc = RelationGetDescr(testrel); + TupleTableSlot *slot; + TableScanDesc scan; + Snapshot snapshot; + + /* Scan all tuples in this relation */ + snapshot = RegisterSnapshot(GetLatestSnapshot()); + scan = table_beginscan(testrel, snapshot, 0, NULL); + slot = table_slot_create(testrel, NULL); + while (table_scan_getnextslot(scan, ForwardScanDirection, slot)) + { + int i; + + /* Test attributes that are of the domain */ + for (i = 0; i < rtc->natts; i++) + { + int attnum = rtc->atts[i]; + Form_pg_attribute attr = TupleDescAttr(tupdesc, attnum - 1); + + if (slot_attisnull(slot, attnum)) + { + /* + * In principle the auxiliary information for this + * error should be errdatatype(), but errtablecol() + * seems considerably more useful in practice. Since + * this code only executes in an ALTER DOMAIN command, + * the client should already know which domain is in + * question. + */ + ereport(ERROR, + (errcode(ERRCODE_NOT_NULL_VIOLATION), + errmsg("column \"%s\" of table \"%s\" contains null values", + NameStr(attr->attname), + RelationGetRelationName(testrel)), + errtablecol(testrel, attnum))); + } + } + } + ExecDropSingleTupleTableSlot(slot); + table_endscan(scan); + UnregisterSnapshot(snapshot); + + /* Close each rel after processing, but keep lock */ + table_close(testrel, NoLock); + } +} + +static void +validateDomainCheckConstraint(Oid domainoid, char *ccbin) { Expr *expr = (Expr *) stringToNode(ccbin); List *rels; @@ -3429,12 +3490,12 @@ checkDomainOwner(HeapTuple tup) } /* - * domainAddConstraint - code shared between CREATE and ALTER DOMAIN + * domainAddCheckConstraint - code shared between CREATE and ALTER DOMAIN */ static char * -domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, - int typMod, Constraint *constr, - const char *domainName, ObjectAddress *constrAddr) +domainAddCheckConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, + int typMod, Constraint *constr, + const char *domainName, ObjectAddress *constrAddr) { Node *expr; char *ccbin; @@ -3442,6 +3503,8 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, CoerceToDomainValue *domVal; Oid ccoid; + Assert(constr->contype == CONSTR_CHECK); + /* * Assign or validate constraint name */ @@ -3561,7 +3624,7 @@ replace_domain_constraint_value(ParseState *pstate, ColumnRef *cref) { /* * Check for a reference to "value", and if that's what it is, replace - * with a CoerceToDomainValue as prepared for us by domainAddConstraint. + * with a CoerceToDomainValue as prepared for us by domainAddCheckConstraint. * (We handle VALUE as a name, not a keyword, to avoid breaking a lot of * applications that have used VALUE as a column name in the past.) */ @@ -3583,6 +3646,78 @@ replace_domain_constraint_value(ParseState *pstate, ColumnRef *cref) return NULL; } +/* + * domainAddNotNullConstraint - code shared between CREATE and ALTER DOMAIN + */ +static void +domainAddNotNullConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, + int typMod, Constraint *constr, + const char *domainName, ObjectAddress *constrAddr) +{ + Oid ccoid; + + Assert(constr->contype == CONSTR_NOTNULL); + + /* + * Assign or validate constraint name + */ + if (constr->conname) + { + if (ConstraintNameIsUsed(CONSTRAINT_DOMAIN, + domainOid, + constr->conname)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("constraint \"%s\" for domain \"%s\" already exists", + constr->conname, domainName))); + } + else + constr->conname = ChooseConstraintName(domainName, + NULL, + "not_null", + domainNamespace, + NIL); + + /* + * Store the constraint in pg_constraint + */ + ccoid = + CreateConstraintEntry(constr->conname, /* Constraint Name */ + domainNamespace, /* namespace */ + CONSTRAINT_NOTNULL, /* Constraint Type */ + false, /* Is Deferrable */ + false, /* Is Deferred */ + !constr->skip_validation, /* Is Validated */ + InvalidOid, /* no parent constraint */ + InvalidOid, /* not a relation constraint */ + NULL, + 0, + 0, + domainOid, /* domain constraint */ + InvalidOid, /* no associated index */ + InvalidOid, /* Foreign key fields */ + NULL, + NULL, + NULL, + NULL, + 0, + ' ', + ' ', + NULL, + 0, + ' ', + NULL, /* not an exclusion constraint */ + NULL, + NULL, + true, /* is local */ + 0, /* inhcount */ + false, /* connoinherit */ + false); /* is_internal */ + + if (constrAddr) + ObjectAddressSet(*constrAddr, ConstraintRelationId, ccoid); +} + /* * Execute ALTER TYPE RENAME diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 608cd5e8e4..ee1adff932 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -1068,7 +1068,7 @@ load_domaintype_info(TypeCacheEntry *typentry) Expr *check_expr; DomainConstraintState *r; - /* Ignore non-CHECK constraints (presently, shouldn't be any) */ + /* Ignore non-CHECK constraints */ if (c->contype != CONSTRAINT_CHECK) continue; diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 34fd0a86e9..16e9528d6a 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7552,7 +7552,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) "pg_catalog.pg_get_constraintdef(oid) AS consrc, " "convalidated " "FROM pg_catalog.pg_constraint " - "WHERE contypid = $1 " + "WHERE contypid = $1 AND contype = 'c' " "ORDER BY conname"); ExecuteSqlStatement(fout, query->data); diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h index a026b42515..c007fe81a8 100644 --- a/src/include/catalog/pg_constraint.h +++ b/src/include/catalog/pg_constraint.h @@ -247,6 +247,7 @@ extern char *ChooseConstraintName(const char *name1, const char *name2, extern HeapTuple findNotNullConstraintAttnum(Oid relid, AttrNumber attnum); extern HeapTuple findNotNullConstraint(Oid relid, const char *colname); +extern HeapTuple findDomainNotNullConstraint(Oid typid); extern AttrNumber extractNotNullColumn(HeapTuple constrTup); extern bool AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count, bool is_no_inherit); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index e70aebd70c..799b268ac7 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -798,6 +798,29 @@ alter domain con drop constraint nonexistent; ERROR: constraint "nonexistent" of domain "con" does not exist alter domain con drop constraint if exists nonexistent; NOTICE: constraint "nonexistent" of domain "con" does not exist, skipping +-- not-null constraints +create domain connotnull integer; +create table domconnotnulltest +( col1 connotnull +, col2 connotnull +); +insert into domconnotnulltest default values; +alter domain connotnull add not null value; -- fails +ERROR: column "col1" of table "domconnotnulltest" contains null values +update domconnotnulltest set col1 = 5; +alter domain connotnull add not null value; -- fails +ERROR: column "col2" of table "domconnotnulltest" contains null values +update domconnotnulltest set col2 = 6; +alter domain connotnull add constraint constr1 not null value; +update domconnotnulltest set col1 = null; -- fails +ERROR: domain connotnull does not allow null values +alter domain connotnull drop constraint constr1; +update domconnotnulltest set col1 = null; +drop domain connotnull cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to column col2 of table domconnotnulltest +drop cascades to column col1 of table domconnotnulltest +drop table domconnotnulltest; -- Test ALTER DOMAIN .. CONSTRAINT .. NOT VALID create domain things AS INT; CREATE TABLE thethings (stuff things); @@ -1223,12 +1246,13 @@ SELECT * FROM information_schema.column_domain_usage SELECT * FROM information_schema.domain_constraints WHERE domain_name IN ('con', 'dom', 'pos_int', 'things') ORDER BY constraint_name; - constraint_catalog | constraint_schema | constraint_name | domain_catalog | domain_schema | domain_name | is_deferrable | initially_deferred ---------------------+-------------------+-----------------+----------------+---------------+-------------+---------------+-------------------- - regression | public | con_check | regression | public | con | NO | NO - regression | public | meow | regression | public | things | NO | NO - regression | public | pos_int_check | regression | public | pos_int | NO | NO -(3 rows) + constraint_catalog | constraint_schema | constraint_name | domain_catalog | domain_schema | domain_name | is_deferrable | initially_deferred +--------------------+-------------------+------------------+----------------+---------------+-------------+---------------+-------------------- + regression | public | con_check | regression | public | con | NO | NO + regression | public | meow | regression | public | things | NO | NO + regression | public | pos_int_check | regression | public | pos_int | NO | NO + regression | public | pos_int_not_null | regression | public | pos_int | NO | NO +(4 rows) SELECT * FROM information_schema.domains WHERE domain_name IN ('con', 'dom', 'pos_int', 'things') @@ -1247,10 +1271,11 @@ SELECT * FROM information_schema.check_constraints FROM information_schema.domain_constraints WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')) ORDER BY constraint_name; - constraint_catalog | constraint_schema | constraint_name | check_clause ---------------------+-------------------+-----------------+-------------- - regression | public | con_check | (VALUE > 0) - regression | public | meow | (VALUE < 11) - regression | public | pos_int_check | (VALUE > 0) -(3 rows) + constraint_catalog | constraint_schema | constraint_name | check_clause +--------------------+-------------------+------------------+------------------- + regression | public | con_check | (VALUE > 0) + regression | public | meow | (VALUE < 11) + regression | public | pos_int_check | (VALUE > 0) + regression | public | pos_int_not_null | VALUE IS NOT NULL +(4 rows) diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 813048c19f..0e71f04004 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -469,6 +469,32 @@ alter domain con drop constraint nonexistent; alter domain con drop constraint if exists nonexistent; +-- not-null constraints +create domain connotnull integer; +create table domconnotnulltest +( col1 connotnull +, col2 connotnull +); + +insert into domconnotnulltest default values; +alter domain connotnull add not null value; -- fails + +update domconnotnulltest set col1 = 5; +alter domain connotnull add not null value; -- fails + +update domconnotnulltest set col2 = 6; + +alter domain connotnull add constraint constr1 not null value; + +update domconnotnulltest set col1 = null; -- fails + +alter domain connotnull drop constraint constr1; + +update domconnotnulltest set col1 = null; + +drop domain connotnull cascade; +drop table domconnotnulltest; + -- Test ALTER DOMAIN .. CONSTRAINT .. NOT VALID create domain things AS INT; CREATE TABLE thethings (stuff things); -- 2.42.1