Catalog domain not-null constraints
This patch set applies the explicit catalog representation of not-null
constraints introduced by b0e96f3119 for table constraints also to
domain not-null constraints.
Since there is no inheritance or primary keys etc., this is much simpler
and just applies the existing infrastructure to domains as well. As a
result, domain not-null constraints now appear in the information schema
correctly. Another effect is that you can now use the ALTER DOMAIN ...
ADD/DROP CONSTRAINT syntax for not-null constraints as well. This makes
everything consistent overall.
For the most part, I structured the code so that there are now separate
sibling subroutines for domain check constraints and domain not-null
constraints. This seemed to work out best, but one could also consider
other ways to refactor this.
Attachments:
v1-0001-Add-tests-for-domain-related-information-schema-v.patchtext/plain; charset=UTF-8; name=v1-0001-Add-tests-for-domain-related-information-schema-v.patchDownload
From c76dd1e21fb2c0a35d45106649788afe2b2b59bd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 23 Nov 2023 07:35:32 +0100
Subject: [PATCH v1 1/2] Add tests for domain-related information schema views
---
src/test/regress/expected/domain.out | 47 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 24 ++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 6d94e84414..e70aebd70c 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1207,3 +1207,50 @@ create domain testdomain1 as int constraint unsigned check (value > 0);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+--
+-- Information schema
+--
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | table_catalog | table_schema | table_name | column_name
+----------------+---------------+-------------+---------------+--------------+------------+-------------
+ regression | public | con | regression | public | domcontest | col1
+ regression | public | dom | regression | public | domview | col1
+ regression | public | things | regression | public | thethings | stuff
+(3 rows)
+
+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)
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | data_type | character_maximum_length | character_octet_length | character_set_catalog | character_set_schema | character_set_name | collation_catalog | collation_schema | collation_name | numeric_precision | numeric_precision_radix | numeric_scale | datetime_precision | interval_type | interval_precision | domain_default | udt_catalog | udt_schema | udt_name | scope_catalog | scope_schema | scope_name | maximum_cardinality | dtd_identifier
+----------------+---------------+-------------+-----------+--------------------------+------------------------+-----------------------+----------------------+--------------------+-------------------+------------------+----------------+-------------------+-------------------------+---------------+--------------------+---------------+--------------------+----------------+-------------+------------+----------+---------------+--------------+------------+---------------------+----------------
+ regression | public | con | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | dom | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | pos_int | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | things | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+(4 rows)
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ 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)
+
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 745f5d5fd2..813048c19f 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -809,3 +809,27 @@ CREATE TABLE thethings (stuff things);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+
+
+--
+-- Information schema
+--
+
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY constraint_name;
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
+ ORDER BY constraint_name;
base-commit: 414e75540f058b23377219586abb3008507f7099
--
2.42.1
v1-0002-Catalog-domain-not-null-constraints.patchtext/plain; charset=UTF-8; name=v1-0002-Catalog-domain-not-null-constraints.patchDownload
From 778fd05ba7fde5062e64addc98eda5505ef475ca Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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
Hi,
This patch set applies the explicit catalog representation of not-null
constraints introduced by b0e96f3119 for table constraints also to
domain not-null constraints.
Interestingly enough according to the documentation this syntax is
already supported [1]https://www.postgresql.org/docs/current/sql-alterdomain.html[2]https://www.postgresql.org/docs/current/sql-createdomain.html, but the actual query will fail on `master`:
```
=# create domain connotnull integer;
CREATE DOMAIN
=# alter domain connotnull add not null value;
ERROR: unrecognized constraint subtype: 1
```
I wonder if we should reflect this limitation in the documentation
and/or show better error messages. This could be quite surprising to
the user. However if we change the documentation on the `master`
branch this patch will have to change it back.
I was curious about the semantic difference between `SET NOT NULL` and
`ADD NOT NULL value`. When I wanted to figure this out I discovered
something that seems to be a bug:
```
=# create domain connotnull1 integer;
=# create domain connotnull2 integer;
=# alter domain connotnull1 add not null value;
=# alter domain connotnull2 set not null;
=# \dD
ERROR: unexpected null value in cached tuple for catalog
pg_constraint column conkey
```
Also it turned out that I can do both: `SET NOT NULL` and `ADD NOT
NULL value` for the same domain. Is it an intended behavior? We should
either forbid it or cover this case with a test.
NOT VALID is not supported:
```
=# alter domain connotnull add not null value not valid;
ERROR: NOT NULL constraints cannot be marked NOT VALID
```
... and this is correct: "NOT VALID is only accepted for CHECK
constraints" [1]https://www.postgresql.org/docs/current/sql-alterdomain.html. This code path however doesn't seem to be
test-covered even on `master`. While on it, I suggest fixing this.
[1]: https://www.postgresql.org/docs/current/sql-alterdomain.html
[2]: https://www.postgresql.org/docs/current/sql-createdomain.html
--
Best regards,
Aleksander Alekseev
On 2023-Nov-23, Aleksander Alekseev wrote:
Interestingly enough according to the documentation this syntax is
already supported [1][2], but the actual query will fail on `master`:```
=# create domain connotnull integer;
CREATE DOMAIN
=# alter domain connotnull add not null value;
ERROR: unrecognized constraint subtype: 1
```
Hah, nice ... this only fails in this way on master, though, as a
side-effect of previous NOT NULL work during this cycle. So if we take
Peter's patch, we don't need to worry about it. In 16 it behaves
properly, with a normal syntax error.
```
=# create domain connotnull1 integer;
=# create domain connotnull2 integer;
=# alter domain connotnull1 add not null value;
=# alter domain connotnull2 set not null;
=# \dD
ERROR: unexpected null value in cached tuple for catalog
pg_constraint column conkey
```
This is also a master-only problem, as "add not null" is rejected in 16
with a syntax error (and obviously \dD doesn't fail).
NOT VALID is not supported:
```
=# alter domain connotnull add not null value not valid;
ERROR: NOT NULL constraints cannot be marked NOT VALID
```
Yeah, it'll take more work to let NOT NULL constraints be marked NOT
VALID, both on domains and on tables. It'll be a good feature for sure.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La victoria es para quien se atreve a estar solo"
On 2023-Nov-23, Peter Eisentraut wrote:
This patch set applies the explicit catalog representation of not-null
constraints introduced by b0e96f3119 for table constraints also to domain
not-null constraints.
I like the idea of having domain not-null constraints appear in
pg_constraint.
Since there is no inheritance or primary keys etc., this is much simpler and
just applies the existing infrastructure to domains as well.
If you create a table with column of domain that has a NOT NULL
constraint, what happens? I mean, is the table column marked
attnotnull, and how does it behave? Is there a separate pg_constraint
row for the constraint in the table? What happens if you do
ALTER TABLE ... DROP NOT NULL for that column?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Por suerte hoy explotó el califont porque si no me habría muerto
de aburrido" (Papelucho)
On 23.11.23 17:38, Alvaro Herrera wrote:
If you create a table with column of domain that has a NOT NULL
constraint, what happens? I mean, is the table column marked
attnotnull, and how does it behave?
No, the domain does not affect the catalog entry for the column. This
is the same way it behaves now.
Is there a separate pg_constraint
row for the constraint in the table? What happens if you do
ALTER TABLE ... DROP NOT NULL for that column?
Those are separate. After dropping the NOT NULL for a column, null
values for the column could still be rejected by a domain. (This is the
same way CHECK constraints work.)
On 23.11.23 14:13, Aleksander Alekseev wrote:
=# create domain connotnull1 integer;
=# create domain connotnull2 integer;
=# alter domain connotnull1 add not null value;
=# alter domain connotnull2 set not null;
=# \dD
ERROR: unexpected null value in cached tuple for catalog
pg_constraint column conkey
Yeah, for domain not-null constraints pg_constraint.conkey is indeed
null. Should we put something in there?
Attached is an updated patch that avoids the error by taking a separate
code path for domain constraints in ruleutils.c. But maybe there is
another way to arrange this.
Attachments:
v2-0002-Catalog-domain-not-null-constraints.patchtext/plain; charset=UTF-8; name=v2-0002-Catalog-domain-not-null-constraints.patchDownload
From c297c458766a0e1ee65408d3ced469f32cf5e7d8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 28 Nov 2023 20:38:16 +0100
Subject: [PATCH v2 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
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/backend/catalog/information_schema.sql | 2 +-
src/backend/catalog/pg_constraint.c | 40 +++
src/backend/commands/typecmds.c | 313 +++++++++++++++------
src/backend/utils/adt/ruleutils.c | 8 +
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 ++
9 files changed, 339 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/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index ed7f40f053..ef39e30ca9 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2492,6 +2492,8 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
}
case CONSTRAINT_NOTNULL:
{
+ if (conForm->conrelid)
+ {
AttrNumber attnum;
attnum = extractNotNullColumn(tup);
@@ -2501,6 +2503,12 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
attnum, false)));
if (((Form_pg_constraint) GETSTRUCT(tup))->connoinherit)
appendStringInfoString(&buf, " NO INHERIT");
+ }
+ else if (conForm->contypid)
+ {
+ /* conkey is null for domain not-null constraints */
+ appendStringInfoString(&buf, "NOT NULL VALUE");
+ }
break;
}
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.43.0
v2-0001-Add-tests-for-domain-related-information-schema-v.patchtext/plain; charset=UTF-8; name=v2-0001-Add-tests-for-domain-related-information-schema-v.patchDownload
From d0cc34bbcc893d8c84331235d181cdc2766974d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 23 Nov 2023 07:35:32 +0100
Subject: [PATCH v2 1/2] Add tests for domain-related information schema views
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/test/regress/expected/domain.out | 47 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 24 ++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 6d94e84414..e70aebd70c 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1207,3 +1207,50 @@ create domain testdomain1 as int constraint unsigned check (value > 0);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+--
+-- Information schema
+--
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | table_catalog | table_schema | table_name | column_name
+----------------+---------------+-------------+---------------+--------------+------------+-------------
+ regression | public | con | regression | public | domcontest | col1
+ regression | public | dom | regression | public | domview | col1
+ regression | public | things | regression | public | thethings | stuff
+(3 rows)
+
+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)
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | data_type | character_maximum_length | character_octet_length | character_set_catalog | character_set_schema | character_set_name | collation_catalog | collation_schema | collation_name | numeric_precision | numeric_precision_radix | numeric_scale | datetime_precision | interval_type | interval_precision | domain_default | udt_catalog | udt_schema | udt_name | scope_catalog | scope_schema | scope_name | maximum_cardinality | dtd_identifier
+----------------+---------------+-------------+-----------+--------------------------+------------------------+-----------------------+----------------------+--------------------+-------------------+------------------+----------------+-------------------+-------------------------+---------------+--------------------+---------------+--------------------+----------------+-------------+------------+----------+---------------+--------------+------------+---------------------+----------------
+ regression | public | con | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | dom | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | pos_int | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | things | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+(4 rows)
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ 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)
+
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 745f5d5fd2..813048c19f 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -809,3 +809,27 @@ CREATE TABLE thethings (stuff things);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+
+
+--
+-- Information schema
+--
+
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY constraint_name;
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
+ ORDER BY constraint_name;
--
2.43.0
On Wed, 29 Nov 2023 at 01:14, Peter Eisentraut <peter@eisentraut.org> wrote:
On 23.11.23 14:13, Aleksander Alekseev wrote:
=# create domain connotnull1 integer;
=# create domain connotnull2 integer;
=# alter domain connotnull1 add not null value;
=# alter domain connotnull2 set not null;
=# \dD
ERROR: unexpected null value in cached tuple for catalog
pg_constraint column conkeyYeah, for domain not-null constraints pg_constraint.conkey is indeed
null. Should we put something in there?Attached is an updated patch that avoids the error by taking a separate
code path for domain constraints in ruleutils.c. But maybe there is
another way to arrange this.
One of the test has failed in CFBot at [1] with:
diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
/tmp/cirrus-ci-build/src/test/regress/results/domain.out
--- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out
2024-01-14 15:40:01.793434601 +0000
+++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out
2024-01-14 15:42:23.013332625 +0000
@@ -1271,11 +1271,4 @@
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)
- regression | public | pos_int_not_null | VALUE IS NOT NULL
-(4 rows)
-
+ERROR: could not open relation with OID 36379
[1]: https://cirrus-ci.com/task/4536440638406656
[2]: https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffs
Regards,
Vignesh
On 17.01.24 13:15, vignesh C wrote:
One of the test has failed in CFBot at [1] with: diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out /tmp/cirrus-ci-build/src/test/regress/results/domain.out --- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out 2024-01-14 15:40:01.793434601 +0000 +++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out 2024-01-14 15:42:23.013332625 +0000 @@ -1271,11 +1271,4 @@ 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) - regression | public | pos_int_not_null | VALUE IS NOT NULL -(4 rows) - +ERROR: could not open relation with OID 36379[1] - https://cirrus-ci.com/task/4536440638406656
[2] - https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffs
Interesting. I couldn't reproduce this locally, even across different
operating systems. The cfbot failures appear to be sporadic, but also
happening across multiple systems, so it's clearly not just a local
environment failure. Can anyone else perhaps reproduce this locally?
On 18.01.24 07:53, Peter Eisentraut wrote:
On 17.01.24 13:15, vignesh C wrote:
One of the test has failed in CFBot at [1] with: diff -U3 /tmp/cirrus-ci-build/src/test/regress/expected/domain.out /tmp/cirrus-ci-build/src/test/regress/results/domain.out --- /tmp/cirrus-ci-build/src/test/regress/expected/domain.out 2024-01-14 15:40:01.793434601 +0000 +++ /tmp/cirrus-ci-build/src/test/regress/results/domain.out 2024-01-14 15:42:23.013332625 +0000 @@ -1271,11 +1271,4 @@ 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) - regression | public | pos_int_not_null | VALUE IS NOT NULL -(4 rows) - +ERROR: could not open relation with OID 36379[1] - https://cirrus-ci.com/task/4536440638406656
[2] -
https://api.cirrus-ci.com/v1/artifact/task/4536440638406656/log/src/test/regress/regression.diffsInteresting. I couldn't reproduce this locally, even across different
operating systems. The cfbot failures appear to be sporadic, but also
happening across multiple systems, so it's clearly not just a local
environment failure. Can anyone else perhaps reproduce this locally?
This patch set needed a rebase, so here it is.
About the sporadic test failure above, I think that is an existing issue
that is just now exposed through some test timing changes. The
pg_get_expr() function used in information_schema.check_constraints has
no locking against concurrent drops of tables. I think in this
particular case, the tests "domain" and "alter_table" are prone to this
conflict. If I move "domain" to a separate test group, the issue goes
away. I'll start a separate discussion about this issue.
Attachments:
v3-0001-Add-tests-for-domain-related-information-schema-v.patchtext/plain; charset=UTF-8; name=v3-0001-Add-tests-for-domain-related-information-schema-v.patchDownload
From cd48c8eb8aa5958020e899b8406cbd7866805db5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 23 Nov 2023 07:35:32 +0100
Subject: [PATCH v3 1/2] Add tests for domain-related information schema views
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/test/regress/expected/domain.out | 47 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 24 ++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 6d94e84414a..e70aebd70c0 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1207,3 +1207,50 @@ create domain testdomain1 as int constraint unsigned check (value > 0);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+--
+-- Information schema
+--
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | table_catalog | table_schema | table_name | column_name
+----------------+---------------+-------------+---------------+--------------+------------+-------------
+ regression | public | con | regression | public | domcontest | col1
+ regression | public | dom | regression | public | domview | col1
+ regression | public | things | regression | public | thethings | stuff
+(3 rows)
+
+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)
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | data_type | character_maximum_length | character_octet_length | character_set_catalog | character_set_schema | character_set_name | collation_catalog | collation_schema | collation_name | numeric_precision | numeric_precision_radix | numeric_scale | datetime_precision | interval_type | interval_precision | domain_default | udt_catalog | udt_schema | udt_name | scope_catalog | scope_schema | scope_name | maximum_cardinality | dtd_identifier
+----------------+---------------+-------------+-----------+--------------------------+------------------------+-----------------------+----------------------+--------------------+-------------------+------------------+----------------+-------------------+-------------------------+---------------+--------------------+---------------+--------------------+----------------+-------------+------------+----------+---------------+--------------+------------+---------------------+----------------
+ regression | public | con | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | dom | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | pos_int | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | things | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+(4 rows)
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ 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)
+
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 745f5d5fd2b..813048c19f5 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -809,3 +809,27 @@ CREATE TABLE thethings (stuff things);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+
+
+--
+-- Information schema
+--
+
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY constraint_name;
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
+ ORDER BY constraint_name;
base-commit: b83033c3cff556d520281aaec399e47b4f11edbe
--
2.43.0
v3-0002-Catalog-domain-not-null-constraints.patchtext/plain; charset=UTF-8; name=v3-0002-Catalog-domain-not-null-constraints.patchDownload
From d34881a605d43241686816aa00603aa1b4a55179 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 6 Feb 2024 14:55:54 +0100
Subject: [PATCH v3 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
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/backend/catalog/information_schema.sql | 2 +-
src/backend/catalog/pg_constraint.c | 40 +++
src/backend/commands/typecmds.c | 320 +++++++++++++++------
src/backend/utils/adt/ruleutils.c | 22 +-
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 ++
9 files changed, 351 insertions(+), 113 deletions(-)
diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index c402ae72741..76c78c0d184 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -445,7 +445,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 0a95608179d..59ca40b7c23 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -631,6 +631,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 a400fb39f67..42ee58aab70 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);
+ domainAddNotNullConstraint(domainoid, typTup->typnamespace,
+ typTup->typbasetype, typTup->typtypmod,
+ constr, NameStr(typTup->typname), NULL);
- 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;
+ validateDomainNotNullConstraint(domainoid);
+ }
+ else
+ {
+ HeapTuple conTup;
+ ObjectAddress conobj;
- /* 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);
+ conTup = findDomainNotNullConstraint(domainoid);
+ if (conTup == NULL)
+ elog(ERROR, "could not find not-null constraint on domain \"%s\"", NameStr(typTup->typname));
- 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);
- }
+ 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,46 @@ 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 +3097,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 +3124,68 @@ 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 +3491,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 +3504,8 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
CoerceToDomainValue *domVal;
Oid ccoid;
+ Assert(constr->contype == CONSTR_CHECK);
+
/*
* Assign or validate constraint name
*/
@@ -3562,9 +3626,10 @@ 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.
- * (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.)
+ * 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.)
*/
if (list_length(cref->fields) == 1)
{
@@ -3584,6 +3649,79 @@ 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, /* conwithoutoverlaps */
+ false); /* is_internal */
+
+ if (constrAddr)
+ ObjectAddressSet(*constrAddr, ConstraintRelationId, ccoid);
+}
+
/*
* Execute ALTER TYPE RENAME
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b625f471a84..e57bbcdbc05 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2494,15 +2494,23 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
}
case CONSTRAINT_NOTNULL:
{
- AttrNumber attnum;
+ if (conForm->conrelid)
+ {
+ AttrNumber attnum;
- attnum = extractNotNullColumn(tup);
+ 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");
+ appendStringInfo(&buf, "NOT NULL %s",
+ quote_identifier(get_attname(conForm->conrelid,
+ attnum, false)));
+ if (((Form_pg_constraint) GETSTRUCT(tup))->connoinherit)
+ appendStringInfoString(&buf, " NO INHERIT");
+ }
+ else if (conForm->contypid)
+ {
+ /* conkey is null for domain not-null constraints */
+ appendStringInfoString(&buf, "NOT NULL VALUE");
+ }
break;
}
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 84fc83cb11f..2d689242b65 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1069,7 +1069,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 348748bae53..6261d3e0fb7 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7831,7 +7831,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 01e6bc21cd1..4e738b50b2e 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -257,6 +257,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 e70aebd70c0..799b268ac76 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 813048c19f5..0e71f040040 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.43.0
On Wed, Feb 7, 2024 at 4:11 PM Peter Eisentraut <peter@eisentraut.org> wrote:
Interesting. I couldn't reproduce this locally, even across different
operating systems. The cfbot failures appear to be sporadic, but also
happening across multiple systems, so it's clearly not just a local
environment failure. Can anyone else perhaps reproduce this locally?This patch set needed a rebase, so here it is.
do you think
add following
ALTER DOMAIN <replaceable class="parameter">name</replaceable> ADD NOT
NULL VALUE
to doc/src/sgml/ref/alter_domain.sgml synopsis makes sense?
otherwise it would be hard to find out this command, i think.
I think I found a bug.
connotnull already set to not null.
every execution of `alter domain connotnull add not null value ;`
would concatenate 'NOT NULL VALUE' for the "Check" column,
That means changes in the function pg_get_constraintdef_worker are not
100% correct.
see below demo:
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privileges | Description
--------+------------+---------+-----------+----------+---------+----------------+-------------------+-------------
public | connotnull | integer | | | | NOT
NULL VALUE | |
public | nnint | integer | | not null | | NOT
NULL VALUE | |
(2 rows)
src8=# alter domain connotnull add not null value ;
ALTER DOMAIN
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privileges | Descript
ion
--------+------------+---------+-----------+----------+---------+-------------------------------+-------------------+---------
----
public | connotnull | integer | | not null | | NOT
NULL VALUE NOT NULL VALUE | |
public | nnint | integer | | not null | | NOT
NULL VALUE | |
(2 rows)
src8=# alter domain connotnull add not null value ;
ALTER DOMAIN
src8=# \dD+
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check | Access privil
eges | Description
--------+------------+---------+-----------+----------+---------+----------------------------------------------+--------------
-----+-------------
public | connotnull | integer | | not null | | NOT
NULL VALUE NOT NULL VALUE NOT NULL VALUE |
|
public | nnint | integer | | not null | | NOT
NULL VALUE |
|
(2 rows)
On 08.02.24 13:17, jian he wrote:
I think I found a bug.
connotnull already set to not null.
every execution of `alter domain connotnull add not null value ;`
would concatenate 'NOT NULL VALUE' for the "Check" column,
I would have expected that. Each invocation adds a new constraint.
But I see that table constraints do not work that way. A command like
ALTER TABLE t1 ADD NOT NULL c1 does nothing if the column already has a
NOT NULL constraint. I'm not sure this is correct. At least it's not
documented. We should probably make the domains feature work the same
way, but I would like to understand why it works that way first.
Peter Eisentraut <peter@eisentraut.org> writes:
But I see that table constraints do not work that way. A command like
ALTER TABLE t1 ADD NOT NULL c1 does nothing if the column already has a
NOT NULL constraint. I'm not sure this is correct. At least it's not
documented. We should probably make the domains feature work the same
way, but I would like to understand why it works that way first.
That's probably a hangover from when the underlying state was just
a boolean (attnotnull). Still, I'm a little hesitant to change the
behavior. I do agree that named constraints need to "stack", so
that you'd have to remove each one before not-nullness stops being
enforced. Less sure about unnamed properties.
regards, tom lane
On 2024-Feb-11, Peter Eisentraut wrote:
But I see that table constraints do not work that way. A command like ALTER
TABLE t1 ADD NOT NULL c1 does nothing if the column already has a NOT NULL
constraint. I'm not sure this is correct. At least it's not documented.
We should probably make the domains feature work the same way, but I would
like to understand why it works that way first.
It's an intentional design decision actually; I had it creating multiple
constraints at first, but it had some ugly problems, so I made it behave
this way (which was no small amount of changes). I think the first time
I posted an implementation that worked this way was here
/messages/by-id/20230315224440.cz3brr6323fcrxs6@alvherre.pgsql
and then we debated it again later, starting at the bottom of
/messages/by-id/CAEZATCUA_iPo5kqUun4myghoZtgqbY3jx62=GwcYKRMmxFUq_g@mail.gmail.com
A few messages later, I quoted the SQL standard for DROP NOT NULL, which
is pretty clear that if you run that command, then the column becomes
possibly nullable, which means that we'd have to drop all matching
constraints, or something.
The main source of nastiness, when we allow multiple constraints, is
constraint inheritance. If we allow just one constraint per column,
then it's always easy to know what to do on inheritance attach and
detach: just coninhcount+1 or coninhcount-1 of the one relevant
constraint (which can be matched by column name). If we have multiple
ones, we have to know which one(s) to match and how (by constraint
name?); if the parent has two and the child has one, we need to create
another in the child, with its own coninhcount adjustments; if the
parent has one named parent_col_not_null and the child also has
child_col_not_null, then at ADD INHERIT do we match these ignoring the
differing name, or do we rename the one on child so that we now have
two? Also, the clutter in psql/pg_dump becomes worse.
I would suggest that domain not-null constraints should also allow just
one per column.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"People get annoyed when you try to debug them." (Larry Wall)
wandering around the function AlterDomainNotNull,
the following code can fix the previous undesired behavior.
seems pretty simple, am I missing something?
based on v3-0001-Add-tests-for-domain-related-information-schema-v.patch
and v3-0002-Catalog-domain-not-null-constraints.patch
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 2f94e375..9069465a 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -2904,7 +2904,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
Form_pg_type typTup;
Constraint *constr;
char *ccbin;
- ObjectAddress address;
+ ObjectAddress address = InvalidObjectAddress;
/* Make a TypeName so we can use standard type lookup machinery */
typename = makeTypeNameFromNameList(names);
@@ -3003,6 +3003,12 @@ AlterDomainAddConstraint(List *names, Node
*newConstraint,
}
else if (constr->contype == CONSTR_NOTNULL)
{
+ /* Is the domain already set NOT NULL */
+ if (typTup->typnotnull)
+ {
+ table_close(typrel, RowExclusiveLock);
+ return address;
+ }
domainAddNotNullConstraint(domainoid, typTup->typnamespace,
typTup->typbasetype, typTup->typtypmod,
constr, NameStr(typTup->typname), constrAddr);
On 12.02.24 11:24, Alvaro Herrera wrote:
On 2024-Feb-11, Peter Eisentraut wrote:
But I see that table constraints do not work that way. A command like ALTER
TABLE t1 ADD NOT NULL c1 does nothing if the column already has a NOT NULL
constraint. I'm not sure this is correct. At least it's not documented.
We should probably make the domains feature work the same way, but I would
like to understand why it works that way first.
The main source of nastiness, when we allow multiple constraints, is
constraint inheritance. If we allow just one constraint per column,
then it's always easy to know what to do on inheritance attach and
detach: just coninhcount+1 or coninhcount-1 of the one relevant
constraint (which can be matched by column name). If we have multiple
ones, we have to know which one(s) to match and how (by constraint
name?); if the parent has two and the child has one, we need to create
another in the child, with its own coninhcount adjustments; if the
parent has one named parent_col_not_null and the child also has
child_col_not_null, then at ADD INHERIT do we match these ignoring the
differing name, or do we rename the one on child so that we now have
two? Also, the clutter in psql/pg_dump becomes worse.I would suggest that domain not-null constraints should also allow just
one per column.
Perhaps it would make sense if we change the ALTER TABLE command to be like
ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1
Then the behavior is like one would expect.
For ALTER TABLE, we would reject this command if IF NOT EXISTS is not
specified. (Since this is mainly for pg_dump, it doesn't really matter
for usability.) For ALTER DOMAIN, we could accept both variants.
On 2024-Mar-14, Peter Eisentraut wrote:
Perhaps it would make sense if we change the ALTER TABLE command to be like
ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1
Then the behavior is like one would expect.
For ALTER TABLE, we would reject this command if IF NOT EXISTS is not
specified. (Since this is mainly for pg_dump, it doesn't really matter for
usability.) For ALTER DOMAIN, we could accept both variants.
I don't understand why you want to change this behavior, though.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"La victoria es para quien se atreve a estar solo"
On 14.03.24 15:03, Alvaro Herrera wrote:
On 2024-Mar-14, Peter Eisentraut wrote:
Perhaps it would make sense if we change the ALTER TABLE command to be like
ALTER TABLE t1 ADD IF NOT EXISTS NOT NULL c1
Then the behavior is like one would expect.
For ALTER TABLE, we would reject this command if IF NOT EXISTS is not
specified. (Since this is mainly for pg_dump, it doesn't really matter for
usability.) For ALTER DOMAIN, we could accept both variants.I don't understand why you want to change this behavior, though.
Because in the abstract, the behavior of
ALTER TABLE t1 ADD <constraint specification>
should be to add a constraint.
In the current implementation, the behavior is different for different
constraint types.
Anyway, in order to move this forward, here is an updated patch where
the ADD CONSTRAINT ... NOT NULL behavior for domains matches the
idempotent behavior of tables. This uses the patch that Jian He posted.
Attachments:
v4-0001-Add-tests-for-domain-related-information-schema-v.patchtext/plain; charset=UTF-8; name=v4-0001-Add-tests-for-domain-related-information-schema-v.patchDownload
From a0075e4bcd5f2db292f92fc2b70576ccebd07210 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 23 Nov 2023 07:35:32 +0100
Subject: [PATCH v4 1/2] Add tests for domain-related information schema views
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/test/regress/expected/domain.out | 47 ++++++++++++++++++++++++++++
src/test/regress/sql/domain.sql | 24 ++++++++++++++
2 files changed, 71 insertions(+)
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index 6d94e84414a..e70aebd70c0 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -1207,3 +1207,50 @@ create domain testdomain1 as int constraint unsigned check (value > 0);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+--
+-- Information schema
+--
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | table_catalog | table_schema | table_name | column_name
+----------------+---------------+-------------+---------------+--------------+------------+-------------
+ regression | public | con | regression | public | domcontest | col1
+ regression | public | dom | regression | public | domview | col1
+ regression | public | things | regression | public | thethings | stuff
+(3 rows)
+
+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)
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+ domain_catalog | domain_schema | domain_name | data_type | character_maximum_length | character_octet_length | character_set_catalog | character_set_schema | character_set_name | collation_catalog | collation_schema | collation_name | numeric_precision | numeric_precision_radix | numeric_scale | datetime_precision | interval_type | interval_precision | domain_default | udt_catalog | udt_schema | udt_name | scope_catalog | scope_schema | scope_name | maximum_cardinality | dtd_identifier
+----------------+---------------+-------------+-----------+--------------------------+------------------------+-----------------------+----------------------+--------------------+-------------------+------------------+----------------+-------------------+-------------------------+---------------+--------------------+---------------+--------------------+----------------+-------------+------------+----------+---------------+--------------+------------+---------------------+----------------
+ regression | public | con | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | dom | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | pos_int | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+ regression | public | things | integer | | | | | | | | | 32 | 2 | 0 | | | | | regression | pg_catalog | int4 | | | | | 1
+(4 rows)
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ 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)
+
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 745f5d5fd2b..813048c19f5 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -809,3 +809,27 @@ CREATE TABLE thethings (stuff things);
alter domain testdomain1 rename constraint unsigned to unsigned_foo;
alter domain testdomain1 drop constraint unsigned_foo;
drop domain testdomain1;
+
+
+--
+-- Information schema
+--
+
+SELECT * FROM information_schema.column_domain_usage
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY constraint_name;
+
+SELECT * FROM information_schema.domains
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things')
+ ORDER BY domain_name;
+
+SELECT * FROM information_schema.check_constraints
+ WHERE (constraint_schema, constraint_name)
+ IN (SELECT constraint_schema, constraint_name
+ FROM information_schema.domain_constraints
+ WHERE domain_name IN ('con', 'dom', 'pos_int', 'things'))
+ ORDER BY constraint_name;
base-commit: ca108be72e7abf1f801c8e49dcffbbbbf412c0d9
--
2.44.0
v4-0002-Catalog-domain-not-null-constraints.patchtext/plain; charset=UTF-8; name=v4-0002-Catalog-domain-not-null-constraints.patchDownload
From c7e10d25851fa90fe87be84b0d4798fbb110e8fb Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 18 Mar 2024 08:42:12 +0100
Subject: [PATCH v4 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
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
src/backend/catalog/information_schema.sql | 2 +-
src/backend/catalog/pg_constraint.c | 40 +++
src/backend/commands/typecmds.c | 328 +++++++++++++++------
src/backend/utils/adt/ruleutils.c | 22 +-
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 | 62 +++-
src/test/regress/sql/domain.sql | 29 ++
9 files changed, 374 insertions(+), 114 deletions(-)
diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index c402ae72741..76c78c0d184 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -445,7 +445,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 5ea9df219c1..9231bd6027c 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 f4cdec3bf2f..3c6a0f4b589 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);
+ domainAddNotNullConstraint(domainoid, typTup->typnamespace,
+ typTup->typbasetype, typTup->typtypmod,
+ constr, NameStr(typTup->typname), NULL);
- 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;
+ validateDomainNotNullConstraint(domainoid);
+ }
+ else
+ {
+ HeapTuple conTup;
+ ObjectAddress conobj;
- /* 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);
+ conTup = findDomainNotNullConstraint(domainoid);
+ if (conTup == NULL)
+ elog(ERROR, "could not find not-null constraint on domain \"%s\"", NameStr(typTup->typname));
- 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);
- }
+ 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);
@@ -2921,7 +2904,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
Form_pg_type typTup;
Constraint *constr;
char *ccbin;
- ObjectAddress address;
+ ObjectAddress address = InvalidObjectAddress;
/* Make a TypeName so we can use standard type lookup machinery */
typename = makeTypeNameFromNameList(names);
@@ -2947,6 +2930,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint,
switch (constr->contype)
{
case CONSTR_CHECK:
+ case CONSTR_NOTNULL:
/* processed below */
break;
@@ -2989,29 +2973,52 @@ 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)
+ {
+ /* Is the domain already set NOT NULL? */
+ if (typTup->typnotnull)
+ {
+ table_close(typrel, RowExclusiveLock);
+ return address;
+ }
+ 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 +3103,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 +3130,68 @@ 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 +3497,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 +3510,8 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
CoerceToDomainValue *domVal;
Oid ccoid;
+ Assert(constr->contype == CONSTR_CHECK);
+
/*
* Assign or validate constraint name
*/
@@ -3562,9 +3632,10 @@ 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.
- * (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.)
+ * 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.)
*/
if (list_length(cref->fields) == 1)
{
@@ -3584,6 +3655,79 @@ 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, /* conwithoutoverlaps */
+ false); /* is_internal */
+
+ if (constrAddr)
+ ObjectAddressSet(*constrAddr, ConstraintRelationId, ccoid);
+}
+
/*
* Execute ALTER TYPE RENAME
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index f2893d40861..d2de79435d6 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2492,15 +2492,23 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
}
case CONSTRAINT_NOTNULL:
{
- AttrNumber attnum;
+ if (conForm->conrelid)
+ {
+ AttrNumber attnum;
- attnum = extractNotNullColumn(tup);
+ 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");
+ appendStringInfo(&buf, "NOT NULL %s",
+ quote_identifier(get_attname(conForm->conrelid,
+ attnum, false)));
+ if (((Form_pg_constraint) GETSTRUCT(tup))->connoinherit)
+ appendStringInfoString(&buf, " NO INHERIT");
+ }
+ else if (conForm->contypid)
+ {
+ /* conkey is null for domain not-null constraints */
+ appendStringInfoString(&buf, "NOT NULL VALUE");
+ }
break;
}
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index d86c3b06fa0..aa4720cb598 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -1071,7 +1071,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 a5149ca823c..361ae788821 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7845,7 +7845,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 a33b4f17ea8..be408678c22 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -257,6 +257,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 e70aebd70c0..dc58793e3f5 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -798,6 +798,42 @@ 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;
+select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
+ count
+-------
+ 1
+(1 row)
+
+alter domain connotnull add constraint constr1bis not null value; -- redundant
+select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
+ count
+-------
+ 1
+(1 row)
+
+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 +1259,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 +1284,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 813048c19f5..ae1b7fbf97a 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -469,6 +469,35 @@
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;
+select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
+alter domain connotnull add constraint constr1bis not null value; -- redundant
+select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
+
+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.44.0
Hi,
Anyway, in order to move this forward, here is an updated patch where
the ADD CONSTRAINT ... NOT NULL behavior for domains matches the
idempotent behavior of tables. This uses the patch that Jian He posted.
I tested the patch on Raspberry Pi 5 and Intel MacBook and also
experimented with it. Everything seems to work properly.
Personally I believe new functions such as
validateDomainNotNullConstraint() and findDomainNotNullConstraint()
could use a few lines of comments (accepts..., returns..., etc). Also
I think that the commit message should explicitly say that supporting
NOT VALID constraints is out of scope of this patch.
Except for named nitpicks v4 LGTM.
--
Best regards,
Aleksander Alekseev
create domain connotnull integer;
create table domconnotnulltest
( col1 connotnull
, col2 connotnull
);
alter domain connotnull add not null value;
---------------------------
the above query does not work in pg16.
ERROR: syntax error at or near "not".
after applying the patch, now this works.
this new syntax need to be added into the alter_domain.sgml's synopsis and also
need an explanation varlistentry?
+ /*
+ * 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, /* conwithoutoverlaps */
+ false); /* is_internal */
/* conwithoutoverlaps */
should be
/* conperiod */
On 18.03.24 11:02, Aleksander Alekseev wrote:
Hi,
Anyway, in order to move this forward, here is an updated patch where
the ADD CONSTRAINT ... NOT NULL behavior for domains matches the
idempotent behavior of tables. This uses the patch that Jian He posted.I tested the patch on Raspberry Pi 5 and Intel MacBook and also
experimented with it. Everything seems to work properly.Personally I believe new functions such as
validateDomainNotNullConstraint() and findDomainNotNullConstraint()
could use a few lines of comments (accepts..., returns..., etc).
Done.
Also
I think that the commit message should explicitly say that supporting
NOT VALID constraints is out of scope of this patch.
Not done. I don't know what NOT VALID has to do with this. This patch
just changes the internal catalog representation, it doesn't claim to
add or change any features. The documentation already accurately states
that NOT VALID is not supported for NOT NULL constraints.
Except for named nitpicks v4 LGTM.
Committed, thanks.
On 19.03.24 10:57, jian he wrote:
this new syntax need to be added into the alter_domain.sgml's synopsis and also
need an explanation varlistentry?
The ALTER DOMAIN reference page refers to CREATE DOMAIN about the
details of the constraint syntax. I believe this is still accurate. We
could add more detail locally on the ALTER DOMAIN page, but that is not
this patch's job. For example, the details of CHECK constraints are
also not shown on the ALTER DOMAIN page right now.
+ false, /* connoinherit */ + false, /* conwithoutoverlaps */ + false); /* is_internal *//* conwithoutoverlaps */
should be
/* conperiod */
Good catch, thanks.
On Wed, 20 Mar 2024 at 09:43, Peter Eisentraut <peter@eisentraut.org> wrote:
On 19.03.24 10:57, jian he wrote:
this new syntax need to be added into the alter_domain.sgml's synopsis and also
need an explanation varlistentry?The ALTER DOMAIN reference page refers to CREATE DOMAIN about the
details of the constraint syntax. I believe this is still accurate. We
could add more detail locally on the ALTER DOMAIN page, but that is not
this patch's job. For example, the details of CHECK constraints are
also not shown on the ALTER DOMAIN page right now.
Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a
constraint is the same as for CREATE DOMAIN, but that's not the case
for NOT NULL constraints. So, for example, these both work:
CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0);
ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10);
However, for NOT NULL constraints, the ALTER DOMAIN syntax differs
from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be
followed by a column name. So the following CREATE DOMAIN syntax
works:
CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL;
but the equivalent ALTER DOMAIN syntax doesn't work:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
ERROR: syntax error at or near ";"
LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
^
All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".
Looking in the SQL spec, it seems to only mention adding CHECK
constraints to domains, so the option to add NOT NULL constraints
should probably be listed in the "Compatibility" section.
Regards,
Dean
On 20.03.24 12:22, Dean Rasheed wrote:
Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a
constraint is the same as for CREATE DOMAIN, but that's not the case
for NOT NULL constraints. So, for example, these both work:CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0);
ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10);
However, for NOT NULL constraints, the ALTER DOMAIN syntax differs
from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be
followed by a column name. So the following CREATE DOMAIN syntax
works:CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL;
but the equivalent ALTER DOMAIN syntax doesn't work:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
ERROR: syntax error at or near ";"
LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
^All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".
Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. As long as you are only dealing with CHECK
constraints, there is no difference, but it shows up when using NOT NULL
constraint syntax. I agree that this is unsatisfactory. Attached is a
patch to try to sort this out.
Looking in the SQL spec, it seems to only mention adding CHECK
constraints to domains, so the option to add NOT NULL constraints
should probably be listed in the "Compatibility" section.
<canofworms>
A quick reading of the SQL standard suggests to me that the way we are
doing null handling in domain constraints is all wrong. The standard
says that domain constraints are only checked on values that are not
null. So both the handling of constraints using the CHECK syntax is
nonstandard and the existence of explicit NOT NULL constraints is an
extension. The CREATE DOMAIN reference page already explains why all of
this is a bad idea. Do we want to document all of that further, or
maybe we just want to rip out domain not-null constraints, or at least
not add further syntax for it?
</canofworms>
Attachments:
0001-WIP-Fix-ALTER-DOMAIN-NOT-NULL-syntax.patchtext/plain; charset=UTF-8; name=0001-WIP-Fix-ALTER-DOMAIN-NOT-NULL-syntax.patchDownload
From 0bdc4944879670b7b53447833fa9f9b385eb0309 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 21 Mar 2024 12:15:11 +0100
Subject: [PATCH] WIP: Fix ALTER DOMAIN NOT NULL syntax
---
doc/src/sgml/ref/create_domain.sgml | 17 ++++++--
src/backend/parser/gram.y | 60 ++++++++++++++++++++++++++--
src/test/regress/expected/domain.out | 8 ++--
src/test/regress/sql/domain.sql | 8 ++--
4 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/ref/create_domain.sgml b/doc/src/sgml/ref/create_domain.sgml
index 73f9f28d6cf..078bea8410d 100644
--- a/doc/src/sgml/ref/create_domain.sgml
+++ b/doc/src/sgml/ref/create_domain.sgml
@@ -24,9 +24,9 @@
CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replaceable class="parameter">data_type</replaceable>
[ COLLATE <replaceable>collation</replaceable> ]
[ DEFAULT <replaceable>expression</replaceable> ]
- [ <replaceable class="parameter">constraint</replaceable> [ ... ] ]
+ [ <replaceable class="parameter">domain_constraint</replaceable> [ ... ] ]
-<phrase>where <replaceable class="parameter">constraint</replaceable> is:</phrase>
+<phrase>where <replaceable class="parameter">domain_constraint</replaceable> is:</phrase>
[ CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> ]
{ NOT NULL | NULL | CHECK (<replaceable class="parameter">expression</replaceable>) }
@@ -190,7 +190,7 @@ <title>Parameters</title>
</variablelist>
</refsect1>
- <refsect1>
+ <refsect1 id="sql-createdomain-notes">
<title>Notes</title>
<para>
@@ -279,6 +279,17 @@ <title>Compatibility</title>
The command <command>CREATE DOMAIN</command> conforms to the SQL
standard.
</para>
+
+ <para>
+ The syntax <literal>NOT NULL</literal> in this command is a
+ <productname>PostgreSQL</productname> extension. (A standard-conforming
+ way to write the same would be <literal>CHECK (VALUE IS NOT
+ NULL)</literal>. However, per <xref linkend="sql-createdomain-notes"/>,
+ such constraints a best avoided in practice anyway.) The
+ <literal>NULL</literal> <quote>constraint</quote> is a
+ <productname>PostgreSQL</productname> extension (see also <xref
+ linkend="sql-createtable-compatibility"/>).
+ </para>
</refsect1>
<refsect1 id="sql-createdomain-see-also">
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 39a801a1c38..391a708467e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -522,7 +522,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <vsetstmt> generic_set set_rest set_rest_more generic_reset reset_rest
SetResetClause FunctionSetResetClause
-%type <node> TableElement TypedTableElement ConstraintElem TableFuncElement
+%type <node> TableElement TypedTableElement ConstraintElem DomainConstraintElem TableFuncElement
%type <node> columnDef columnOptions
%type <defelt> def_elem reloption_elem old_aggr_elem operator_def_elem
%type <node> def_arg columnElem where_clause where_or_current_clause
@@ -593,7 +593,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <keyword> col_name_keyword reserved_keyword
%type <keyword> bare_label_keyword
-%type <node> TableConstraint TableLikeClause
+%type <node> DomainConstraint TableConstraint TableLikeClause
%type <ival> TableLikeOptionList TableLikeOption
%type <str> column_compression opt_column_compression column_storage opt_column_storage
%type <list> ColQualList
@@ -4251,6 +4251,60 @@ ConstraintElem:
}
;
+/*
+ * DomainConstraint is separate from TableConstraint because the syntax for
+ * NOT NULL constraints is different. For table constraints, we need to
+ * accept a column name, but for domain constraints, we don't. (We could
+ * accept something like NOT NULL VALUE, but that seems weird.) CREATE DOMAIN
+ * (which uses ColQualList) has for a long time accepted NOT NULL without a
+ * column name, so it makes sense that ALTER DOMAIN (which uses
+ * DomainConstraint) does as well. None of these syntaxes are per SQL
+ * standard; we are just living with the bits of inconsistency that have built
+ * up over time.
+ */
+DomainConstraint:
+ CONSTRAINT name DomainConstraintElem
+ {
+ Constraint *n = castNode(Constraint, $3);
+
+ n->conname = $2;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | DomainConstraintElem { $$ = $1; }
+ ;
+
+DomainConstraintElem:
+ CHECK '(' a_expr ')' ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_CHECK;
+ n->location = @1;
+ n->raw_expr = $3;
+ n->cooked_expr = NULL;
+ processCASbits($5, @5, "CHECK",
+ NULL, NULL, &n->skip_validation,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = !n->skip_validation;
+ $$ = (Node *) n;
+ }
+ | NOT NULL_P ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_NOTNULL;
+ n->location = @1;
+ n->keys = list_make1(makeString("value"));
+ /* no NOT VALID support yet */
+ processCASbits($3, @3, "NOT NULL",
+ NULL, NULL, NULL,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = true;
+ $$ = (Node *) n;
+ }
+ ;
+
opt_no_inherit: NO INHERIT { $$ = true; }
| /* EMPTY */ { $$ = false; }
;
@@ -11493,7 +11547,7 @@ AlterDomainStmt:
$$ = (Node *) n;
}
/* ALTER DOMAIN <domain> ADD CONSTRAINT ... */
- | ALTER DOMAIN_P any_name ADD_P TableConstraint
+ | ALTER DOMAIN_P any_name ADD_P DomainConstraint
{
AlterDomainStmt *n = makeNode(AlterDomainStmt);
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index dc58793e3f5..bf5c34bfacf 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -805,20 +805,20 @@ create table domconnotnulltest
, col2 connotnull
);
insert into domconnotnulltest default values;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
ERROR: column "col1" of table "domconnotnulltest" contains null values
update domconnotnulltest set col1 = 5;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
ERROR: column "col2" of table "domconnotnulltest" contains null values
update domconnotnulltest set col2 = 6;
-alter domain connotnull add constraint constr1 not null value;
+alter domain connotnull add constraint constr1 not null;
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
count
-------
1
(1 row)
-alter domain connotnull add constraint constr1bis not null value; -- redundant
+alter domain connotnull add constraint constr1bis not null; -- redundant
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
count
-------
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index ae1b7fbf97a..ce0c2e3cbc2 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -477,16 +477,16 @@
);
insert into domconnotnulltest default values;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
update domconnotnulltest set col1 = 5;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
update domconnotnulltest set col2 = 6;
-alter domain connotnull add constraint constr1 not null value;
+alter domain connotnull add constraint constr1 not null;
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
-alter domain connotnull add constraint constr1bis not null value; -- redundant
+alter domain connotnull add constraint constr1bis not null; -- redundant
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
update domconnotnulltest set col1 = null; -- fails
--
2.44.0
Peter Eisentraut <peter@eisentraut.org> writes:
<canofworms>
A quick reading of the SQL standard suggests to me that the way we are
doing null handling in domain constraints is all wrong. The standard
says that domain constraints are only checked on values that are not
null. So both the handling of constraints using the CHECK syntax is
nonstandard and the existence of explicit NOT NULL constraints is an
extension. The CREATE DOMAIN reference page already explains why all of
this is a bad idea. Do we want to document all of that further, or
maybe we just want to rip out domain not-null constraints, or at least
not add further syntax for it?
</canofworms>
Yeah. The real problem with domain not null is: how can a column
that's propagated up through the nullable side of an outer join
still be considered to belong to such a domain?
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
I'm too lazy to search the archives, but we have had at least one
previous discussion about how we should adopt the spec's semantics.
It'd be an absolutely trivial fix in CoerceToDomain (succeed
immediately if input is NULL), but the question is what to do
with existing "DOMAIN NOT NULL" DDL.
Anyway, now that I recall all that, e5da0fe3c is throwing good work
after bad, and I wonder if we shouldn't revert it.
regards, tom lane
On Thu, 21 Mar 2024 at 10:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
To be fair, NULL is a valid value of every type. Even VOID has NULL.
In this context, it’s a bit weird to be able to decree up front when
defining a type that no table column of that type, anywhere, may ever
contain a NULL. It would be nice if there was a way to reverse the default
so that if you (almost or) never want NULLs anywhere that’s what you get
without saying "NOT NULL" all over the place, and instead just specify
"NULLABLE" (or something) where you want. But that effectively means
optionally changing the behaviour of CREATE TABLE and ALTER TABLE.
On 3/21/24 15:30, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
<canofworms>
A quick reading of the SQL standard suggests to me that the way we are
doing null handling in domain constraints is all wrong. The standard
says that domain constraints are only checked on values that are not
null. So both the handling of constraints using the CHECK syntax is
nonstandard and the existence of explicit NOT NULL constraints is an
extension. The CREATE DOMAIN reference page already explains why all of
this is a bad idea. Do we want to document all of that further, or
maybe we just want to rip out domain not-null constraints, or at least
not add further syntax for it?
</canofworms>Yeah. The real problem with domain not null is: how can a column
that's propagated up through the nullable side of an outer join
still be considered to belong to such a domain?
Per spec, it is not considered to be so. The domain only applies to
table storage and CASTs and gets "forgotten" in a query.
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
I don't see how you can infer this from the standard at all.
I'm too lazy to search the archives, but we have had at least one
previous discussion about how we should adopt the spec's semantics.
It'd be an absolutely trivial fix in CoerceToDomain (succeed
immediately if input is NULL), but the question is what to do
with existing "DOMAIN NOT NULL" DDL.
Here is a semi-random link into a conversation you and I have recently
had about this:
/messages/by-id/a13db59c-c68f-4a30-87a5-177fe135665e@postgresfriends.org
As also said somewhere in that thread, I think that <cast specification>
short-cutting a NULL input value without considering the constraints of
a domain is a bug that needs to be fixed in the standard.
--
Vik Fearing
Vik Fearing <vik@postgresfriends.org> writes:
On 3/21/24 15:30, Tom Lane wrote:
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".
I don't see how you can infer this from the standard at all.
I believe where we got that from is 6.13 <cast specification>,
which quoth (general rule 2):
c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Subclause are applied.
In particular, that short-circuits application of the domain
constraints (GR 23), implying that CAST(NULL AS some_domain) is
always successful. Now you could argue that there's some other
context that would reject nulls, but being inconsistent with
CAST would seem more like a bug than a feature.
As also said somewhere in that thread, I think that <cast specification>
short-cutting a NULL input value without considering the constraints of
a domain is a bug that needs to be fixed in the standard.
I think it's probably intentional. It certainly fits with the lack of
syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99;
do you think nobody's noticed it for 25 years?
regards, tom lane
On 3/22/24 00:17, Tom Lane wrote:
Vik Fearing <vik@postgresfriends.org> writes:
On 3/21/24 15:30, Tom Lane wrote:
The SQL spec's answer to that conundrum appears to be "NULL is
a valid value of every domain, and if you don't like it, tough".I don't see how you can infer this from the standard at all.
I believe where we got that from is 6.13 <cast specification>,
which quoth (general rule 2):c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Subclause are applied.In particular, that short-circuits application of the domain
constraints (GR 23), implying that CAST(NULL AS some_domain) is
always successful. Now you could argue that there's some other
context that would reject nulls, but being inconsistent with
CAST would seem more like a bug than a feature.
I think the main bug is in what you quoted from <cast specification>.
I believe that the POLA for casting to a domain is for all constraints
of the domain to be verified for ALL values including the null value.
As also said somewhere in that thread, I think that <cast specification>
short-cutting a NULL input value without considering the constraints of
a domain is a bug that needs to be fixed in the standard.I think it's probably intentional. It certainly fits with the lack of
syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99;
do you think nobody's noticed it for 25 years?
Haven't we (postgres) had bug reports of similar age?
There is also the possibility that no one has noticed because major
players have not implemented domains. For example, Oracle only just got
them last year:
https://blogs.oracle.com/coretec/post/less-coding-with-sql-domains-in-23c
Anyway, I will bring this up with the committee and report back. My
proposed solution will be for CAST to check domain constraints even if
the input is NULL.
--
Vik Fearing
Vik Fearing <vik@postgresfriends.org> writes:
On 3/22/24 00:17, Tom Lane wrote:
Vik Fearing <vik@postgresfriends.org> writes:
As also said somewhere in that thread, I think that <cast specification>
short-cutting a NULL input value without considering the constraints of
a domain is a bug that needs to be fixed in the standard.
I think it's probably intentional. It certainly fits with the lack of
syntax for DOMAIN NOT NULL. Also, it's been like that since SQL99;
do you think nobody's noticed it for 25 years?
Haven't we (postgres) had bug reports of similar age?
Well, they've looked it at it since then. SQL99 has
c) If SV is the null value, then the result is the null value.
SQL:2008 and later have the text I quoted:
c) If SV is the null value, then the result of CS is the null
value and no further General Rules of this Sub-clause are
applied.
I find it *extremely* hard to believe that they would have added
that explicit text without noticing exactly which operations they
were saying to skip.
Anyway, I will bring this up with the committee and report back. My
proposed solution will be for CAST to check domain constraints even if
the input is NULL.
Please do not claim that that is the position of the Postgres project.
regards, tom lane
On 3/22/24 01:46, Tom Lane wrote:
Vik Fearing <vik@postgresfriends.org> writes:
Anyway, I will bring this up with the committee and report back. My
proposed solution will be for CAST to check domain constraints even if
the input is NULL.Please do not claim that that is the position of the Postgres project.
Everything that I do on the SQL Committee is in my own name.
I do not speak for either PostgreSQL or for EDB (my employer), even
though my opinions are of course often influenced by some combination of
both.
--
Vik Fearing
On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 20.03.24 12:22, Dean Rasheed wrote:
Hmm, for CHECK constraints, the ALTER DOMAIN syntax for adding a
constraint is the same as for CREATE DOMAIN, but that's not the case
for NOT NULL constraints. So, for example, these both work:CREATE DOMAIN d AS int CONSTRAINT c1 CHECK (value > 0);
ALTER DOMAIN d ADD CONSTRAINT c2 CHECK (value < 10);
However, for NOT NULL constraints, the ALTER DOMAIN syntax differs
from the CREATE DOMAIN syntax, because it expects "NOT NULL" to be
followed by a column name. So the following CREATE DOMAIN syntax
works:CREATE DOMAIN d AS int CONSTRAINT nn NOT NULL;
but the equivalent ALTER DOMAIN syntax doesn't work:
ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
ERROR: syntax error at or near ";"
LINE 1: ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL;
^All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. As long as you are only dealing with CHECK
constraints, there is no difference, but it shows up when using NOT NULL
constraint syntax. I agree that this is unsatisfactory. Attached is a
patch to try to sort this out.
+ | NOT NULL_P ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_NOTNULL;
+ n->location = @1;
+ n->keys = list_make1(makeString("value"));
+ /* no NOT VALID support yet */
+ processCASbits($3, @3, "NOT NULL",
+ NULL, NULL, NULL,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = true;
+ $$ = (Node *) n;
+ }
i don't understand this part.
+ n->keys = list_make1(makeString("value"));
also you should also change src/backend/utils/adt/ruleutils.c?
src6=# create domain domain_test integer;
alter domain domain_test add constraint pos1 check (value > 0);
alter domain domain_test add constraint constr1 not null ;
CREATE DOMAIN
ALTER DOMAIN
ALTER DOMAIN
src6=# \dD
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check
--------+-------------+---------+-----------+----------+---------+----------------------------------
public | domain_test | integer | | not null | |
CHECK (VALUE > 0) NOT NULL VALUE
(1 row)
probably change to CHECK (VALUE IS NOT NULL)
- appendStringInfoString(&buf,
"NOT NULL VALUE");
+ appendStringInfoString(&buf,
"CHECK (VALUE IS NOT NULL)");
seems works.
On Fri, 22 Mar 2024 at 08:28, jian he <jian.universality@gmail.com> wrote:
On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote:
Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. Attached is a patch to try to sort this out.also you should also change src/backend/utils/adt/ruleutils.c?
src6=# \dD
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check
--------+-------------+---------+-----------+----------+---------+----------------------------------
public | domain_test | integer | | not null | |
CHECK (VALUE > 0) NOT NULL VALUE
(1 row)probably change to CHECK (VALUE IS NOT NULL)
I'd say it should just output "NOT NULL", since that's the input
syntax that created the constraint. But then again, why display NOT
NULL constraints in that column at all, when there's a separate
"Nullable" column?
Also (not this patch's fault), psql doesn't seem to offer a way to
display domain constraint names -- something you need to know to drop
or alter them. Perhaps \dD+ could be made to do that?
+ The syntax <literal>NOT NULL</literal> in this command is a
+ <productname>PostgreSQL</productname> extension. (A standard-conforming
+ way to write the same would be <literal>CHECK (VALUE IS NOT
+ NULL)</literal>. However, per <xref linkend="sql-createdomain-notes"/>,
+ such constraints a best avoided in practice anyway.) The
+ <literal>NULL</literal> <quote>constraint</quote> is a
+ <productname>PostgreSQL</productname> extension (see also <xref
+ linkend="sql-createtable-compatibility"/>).
I didn't verify this, but I thought that according to the SQL
standard, only non-NULL values should be passed to CHECK constraints,
so there is no standard-conforming way to write a NOT NULL domain
constraint.
FWIW, I think NOT NULL domain constraints are a useful feature to
have, and I suspect that there are more people out there who use them
and like them, than who care what the SQL standard says. If so, I'm in
favour of allowing them to be named and managed in the same way as NOT
NULL table constraints.
+ processCASbits($5, @5, "CHECK",
+ NULL, NULL, &n->skip_validation,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = !n->skip_validation;
+ /* no NOT VALID support yet */
+ processCASbits($3, @3, "NOT NULL",
+ NULL, NULL, NULL,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = true;
NO INHERIT is allowed for domain constraints? What does that even mean?
There's something very wonky about this:
CREATE DOMAIN d1 AS int CHECK (value > 0) NO INHERIT; -- Rejected
ERROR: check constraints for domains cannot be marked NO INHERIT
CREATE DOMAIN d1 AS int;
ALTER DOMAIN d1 ADD CHECK (value > 0) NO INHERIT; -- Allowed
CREATE DOMAIN d2 AS int NOT NULL NO INHERIT; -- Now allowed (used to
syntax error)
CREATE DOMAIN d3 AS int;
ALTER DOMAIN d3 ADD NOT NULL NO INHERIT; -- Allowed
Presumably all of those should be rejected in the grammar.
Regards,
Dean
On 2024-Mar-25, Dean Rasheed wrote:
Also (not this patch's fault), psql doesn't seem to offer a way to
display domain constraint names -- something you need to know to drop
or alter them. Perhaps \dD+ could be made to do that?
Ooh, I remember we had offered a patch for \d++ to display these
constraint names for tables, but didn't get around to gather consensus
for it. We did gather consensus on *not* wanting \d+ to display them,
but we need *something*. I suppose we should do something symmetrical
for tables and domains. How about \dD++ and \dt++?
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree. (Don Knuth)
On Tue, 26 Mar 2024 at 07:30, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2024-Mar-25, Dean Rasheed wrote:
Also (not this patch's fault), psql doesn't seem to offer a way to
display domain constraint names -- something you need to know to drop
or alter them. Perhaps \dD+ could be made to do that?Ooh, I remember we had offered a patch for \d++ to display these
constraint names for tables, but didn't get around to gather consensus
for it. We did gather consensus on *not* wanting \d+ to display them,
but we need *something*. I suppose we should do something symmetrical
for tables and domains. How about \dD++ and \dt++?
Personally, I quite like the fact that \d+ displays NOT NULL
constraints, because it puts them on an equal footing with CHECK
constraints. However, I can appreciate that it will significantly
increase the length of the output in some cases.
With \dD it's not so nice because of the way it puts all the details
on one line. The obvious output might look something like this:
\dD
List of domains
Schema | Name | Type | Collation | Nullable | Default | Check
--------+------+---------+-----------+----------+---------+-------------------
public | d1 | integer | | NOT NULL | | CHECK (VALUE > 0)
\dD+
List of domains
Schema | Name | Type | Collation | Nullable
| Default | Check | Access privileges
| Description
--------+------+---------+-----------+---------------------------------+---------+---------------------------------------+-------------------+-------------
public | d1 | integer | | CONSTRAINT d1_not_null NOT NULL
| | CONSTRAINT d1_check CHECK (VALUE > 0) |
|
So you'd need quite a wide window to easily view it (or use \x). I
suppose the width could be reduced by dropping the word "CONSTRAINT"
in the \dD+ case, but it's probably still going to be wider than the
average window.
Regards,
Dean
On Tue, Mar 26, 2024 at 2:28 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On Fri, 22 Mar 2024 at 08:28, jian he <jian.universality@gmail.com> wrote:
On Thu, Mar 21, 2024 at 7:23 PM Peter Eisentraut <peter@eisentraut.org> wrote:
Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. Attached is a patch to try to sort this out.also you should also change src/backend/utils/adt/ruleutils.c?
src6=# \dD
List of domains
Schema | Name | Type | Collation | Nullable | Default |
Check
--------+-------------+---------+-----------+----------+---------+----------------------------------
public | domain_test | integer | | not null | |
CHECK (VALUE > 0) NOT NULL VALUE
(1 row)probably change to CHECK (VALUE IS NOT NULL)
I'd say it should just output "NOT NULL", since that's the input
syntax that created the constraint. But then again, why display NOT
NULL constraints in that column at all, when there's a separate
"Nullable" column?
create table sss(a int not null);
SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conname =
'sss_a_not_null';
returns
" NOT NULL a"
I think just outputting "NOT NULL" is ok for the domain, given the
table constraint is "NOT NULL" + table column, per above example.
yech, we already have a "Nullable" column, so we don't need to display
NOT NULL constraints.
On 21.03.24 12:23, Peter Eisentraut wrote:
All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. As long as you are only dealing with CHECK
constraints, there is no difference, but it shows up when using NOT NULL
constraint syntax. I agree that this is unsatisfactory. Attached is a
patch to try to sort this out.
After studying this a bit more, I think moving forward in this direction
is the best way. Attached is a new patch version, mainly with a more
elaborate commit message. This patch makes the not-null constraint
syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes
the respective documentation correct.
(Note that, as I show in the commit message, commit e5da0fe3c22 had in
passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just
reverting that commit wouldn't be a complete solution.)
Attachments:
v2-0001-Fix-ALTER-DOMAIN-NOT-NULL-syntax.patchtext/plain; charset=UTF-8; name=v2-0001-Fix-ALTER-DOMAIN-NOT-NULL-syntax.patchDownload
From 4e4de402dda81798bb0286a09d39c6ed2f087228 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 8 Apr 2024 11:39:48 +0200
Subject: [PATCH v2] Fix ALTER DOMAIN NOT NULL syntax
In CREATE DOMAIN, a NOT NULL constraint looks like
CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL
(Before e5da0fe3c22, the constraint name was accepted but ignored.)
But in ALTER DOMAIN, a NOT NULL constraint looks like
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE
where VALUE is where for a table constraint the column name would be.
(This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax
resulted in an internal error.)
But for domains, this latter syntax is confusing and needlessly
inconsistent between CREATE and ALTER. So this changes it to just
ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL
(None of these syntaxes are per SQL standard; we are just living with
the bits of inconsistency that have built up over time.)
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
---
doc/src/sgml/ref/create_domain.sgml | 17 ++++++--
src/backend/parser/gram.y | 60 ++++++++++++++++++++++++++--
src/test/regress/expected/domain.out | 8 ++--
src/test/regress/sql/domain.sql | 8 ++--
4 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/doc/src/sgml/ref/create_domain.sgml b/doc/src/sgml/ref/create_domain.sgml
index 73f9f28d6cf..078bea8410d 100644
--- a/doc/src/sgml/ref/create_domain.sgml
+++ b/doc/src/sgml/ref/create_domain.sgml
@@ -24,9 +24,9 @@
CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replaceable class="parameter">data_type</replaceable>
[ COLLATE <replaceable>collation</replaceable> ]
[ DEFAULT <replaceable>expression</replaceable> ]
- [ <replaceable class="parameter">constraint</replaceable> [ ... ] ]
+ [ <replaceable class="parameter">domain_constraint</replaceable> [ ... ] ]
-<phrase>where <replaceable class="parameter">constraint</replaceable> is:</phrase>
+<phrase>where <replaceable class="parameter">domain_constraint</replaceable> is:</phrase>
[ CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> ]
{ NOT NULL | NULL | CHECK (<replaceable class="parameter">expression</replaceable>) }
@@ -190,7 +190,7 @@ <title>Parameters</title>
</variablelist>
</refsect1>
- <refsect1>
+ <refsect1 id="sql-createdomain-notes">
<title>Notes</title>
<para>
@@ -279,6 +279,17 @@ <title>Compatibility</title>
The command <command>CREATE DOMAIN</command> conforms to the SQL
standard.
</para>
+
+ <para>
+ The syntax <literal>NOT NULL</literal> in this command is a
+ <productname>PostgreSQL</productname> extension. (A standard-conforming
+ way to write the same would be <literal>CHECK (VALUE IS NOT
+ NULL)</literal>. However, per <xref linkend="sql-createdomain-notes"/>,
+ such constraints a best avoided in practice anyway.) The
+ <literal>NULL</literal> <quote>constraint</quote> is a
+ <productname>PostgreSQL</productname> extension (see also <xref
+ linkend="sql-createtable-compatibility"/>).
+ </para>
</refsect1>
<refsect1 id="sql-createdomain-see-also">
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0523f7e891e..e8b619926ef 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -524,7 +524,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <vsetstmt> generic_set set_rest set_rest_more generic_reset reset_rest
SetResetClause FunctionSetResetClause
-%type <node> TableElement TypedTableElement ConstraintElem TableFuncElement
+%type <node> TableElement TypedTableElement ConstraintElem DomainConstraintElem TableFuncElement
%type <node> columnDef columnOptions optionalPeriodName
%type <defelt> def_elem reloption_elem old_aggr_elem operator_def_elem
%type <node> def_arg columnElem where_clause where_or_current_clause
@@ -596,7 +596,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <keyword> col_name_keyword reserved_keyword
%type <keyword> bare_label_keyword
-%type <node> TableConstraint TableLikeClause
+%type <node> DomainConstraint TableConstraint TableLikeClause
%type <ival> TableLikeOptionList TableLikeOption
%type <str> column_compression opt_column_compression column_storage opt_column_storage
%type <list> ColQualList
@@ -4334,6 +4334,60 @@ ConstraintElem:
}
;
+/*
+ * DomainConstraint is separate from TableConstraint because the syntax for
+ * NOT NULL constraints is different. For table constraints, we need to
+ * accept a column name, but for domain constraints, we don't. (We could
+ * accept something like NOT NULL VALUE, but that seems weird.) CREATE DOMAIN
+ * (which uses ColQualList) has for a long time accepted NOT NULL without a
+ * column name, so it makes sense that ALTER DOMAIN (which uses
+ * DomainConstraint) does as well. None of these syntaxes are per SQL
+ * standard; we are just living with the bits of inconsistency that have built
+ * up over time.
+ */
+DomainConstraint:
+ CONSTRAINT name DomainConstraintElem
+ {
+ Constraint *n = castNode(Constraint, $3);
+
+ n->conname = $2;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | DomainConstraintElem { $$ = $1; }
+ ;
+
+DomainConstraintElem:
+ CHECK '(' a_expr ')' ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_CHECK;
+ n->location = @1;
+ n->raw_expr = $3;
+ n->cooked_expr = NULL;
+ processCASbits($5, @5, "CHECK",
+ NULL, NULL, &n->skip_validation,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = !n->skip_validation;
+ $$ = (Node *) n;
+ }
+ | NOT NULL_P ConstraintAttributeSpec
+ {
+ Constraint *n = makeNode(Constraint);
+
+ n->contype = CONSTR_NOTNULL;
+ n->location = @1;
+ n->keys = list_make1(makeString("value"));
+ /* no NOT VALID support yet */
+ processCASbits($3, @3, "NOT NULL",
+ NULL, NULL, NULL,
+ &n->is_no_inherit, yyscanner);
+ n->initially_valid = true;
+ $$ = (Node *) n;
+ }
+ ;
+
opt_no_inherit: NO INHERIT { $$ = true; }
| /* EMPTY */ { $$ = false; }
;
@@ -11586,7 +11640,7 @@ AlterDomainStmt:
$$ = (Node *) n;
}
/* ALTER DOMAIN <domain> ADD CONSTRAINT ... */
- | ALTER DOMAIN_P any_name ADD_P TableConstraint
+ | ALTER DOMAIN_P any_name ADD_P DomainConstraint
{
AlterDomainStmt *n = makeNode(AlterDomainStmt);
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index fa8459e10ff..58e0fc53166 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -805,20 +805,20 @@ create table domconnotnulltest
, col2 connotnull
);
insert into domconnotnulltest default values;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
ERROR: column "col1" of table "domconnotnulltest" contains null values
update domconnotnulltest set col1 = 5;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
ERROR: column "col2" of table "domconnotnulltest" contains null values
update domconnotnulltest set col2 = 6;
-alter domain connotnull add constraint constr1 not null value;
+alter domain connotnull add constraint constr1 not null;
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
count
-------
1
(1 row)
-alter domain connotnull add constraint constr1bis not null value; -- redundant
+alter domain connotnull add constraint constr1bis not null; -- redundant
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
count
-------
diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql
index 763c68f1db6..b9600944fbd 100644
--- a/src/test/regress/sql/domain.sql
+++ b/src/test/regress/sql/domain.sql
@@ -477,16 +477,16 @@
);
insert into domconnotnulltest default values;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
update domconnotnulltest set col1 = 5;
-alter domain connotnull add not null value; -- fails
+alter domain connotnull add not null; -- fails
update domconnotnulltest set col2 = 6;
-alter domain connotnull add constraint constr1 not null value;
+alter domain connotnull add constraint constr1 not null;
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
-alter domain connotnull add constraint constr1bis not null value; -- redundant
+alter domain connotnull add constraint constr1bis not null; -- redundant
select count(*) from pg_constraint where contypid = 'connotnull'::regtype and contype = 'n';
update domconnotnulltest set col1 = null; -- fails
base-commit: bb766cde63b4f624d029b34c9cdd3d0a94fd5b46
--
2.44.0
On Mon, Apr 8, 2024 at 5:53 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 21.03.24 12:23, Peter Eisentraut wrote:
All the examples in the tests append "value" to this, presumably by
analogy with CHECK constraints, but it looks as though anything works,
and is simply ignored:ALTER DOMAIN d ADD CONSTRAINT nn NOT NULL xxx; -- works
That doesn't seem particularly satisfactory. I think it should not
require (and reject) a column name after "NOT NULL".Hmm. CREATE DOMAIN uses column constraint syntax, but ALTER DOMAIN uses
table constraint syntax. As long as you are only dealing with CHECK
constraints, there is no difference, but it shows up when using NOT NULL
constraint syntax. I agree that this is unsatisfactory. Attached is a
patch to try to sort this out.After studying this a bit more, I think moving forward in this direction
is the best way. Attached is a new patch version, mainly with a more
elaborate commit message. This patch makes the not-null constraint
syntax consistent between CREATE DOMAIN and ALTER DOMAIN, and also makes
the respective documentation correct.(Note that, as I show in the commit message, commit e5da0fe3c22 had in
passing fixed a couple of bugs in CREATE and ALTER DOMAIN, so just
reverting that commit wouldn't be a complete solution.)
in ruleutils.c
/* conkey is null for domain not-null constraints */
appendStringInfoString(&buf, "NOT NULL VALUE");
should be
/* conkey is null for domain not-null constraints */
appendStringInfoString(&buf, "NOT NULL ");
?
currently
src6=# \dD connotnull
/******** QUERY *********/
SELECT n.nspname as "Schema",
t.typname as "Name",
pg_catalog.format_type(t.typbasetype, t.typtypmod) as "Type",
(SELECT c.collname FROM pg_catalog.pg_collation c, pg_catalog.pg_type bt
WHERE c.oid = t.typcollation AND bt.oid = t.typbasetype AND
t.typcollation <> bt.typcollation) as "Collation",
CASE WHEN t.typnotnull THEN 'not null' END as "Nullable",
t.typdefault as "Default",
pg_catalog.array_to_string(ARRAY(
SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM
pg_catalog.pg_constraint r WHERE t.oid = r.contypid
), ' ') as "Check"
FROM pg_catalog.pg_type t
LEFT JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace
WHERE t.typtype = 'd'
AND t.typname OPERATOR(pg_catalog.~) '^(connotnull)$' COLLATE
pg_catalog.default
AND pg_catalog.pg_type_is_visible(t.oid)
ORDER BY 1, 2;
/************************/
---
Since the last column is already named as "Check", maybe we need to
change the query to
pg_catalog.array_to_string(ARRAY(
SELECT pg_catalog.pg_get_constraintdef(r.oid, true) FROM
pg_catalog.pg_constraint r WHERE t.oid = r.contypid
and r.contype = 'c'
), ' ') as "Check"
That means domain can be associated with check constraint and not-null
constraint.
the url link destination is fine, but the url rendered name is "per
the section called “Notes”" which seems strange,
please see attached.
Attachments:
Screenshot from 2024-04-09 16-40-57.pngimage/png; name="Screenshot from 2024-04-09 16-40-57.png"Download
�PNG
IHDR
� u8� sBIT|d� tEXtSoftware gnome-screenshot��>