Re: NOT NULL NOT ENFORCED

On Thu, Sep 4, 2025 at 8:00 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2025-Sep-04, jian he wrote:
>
> > @@ -3093,6 +3115,16 @@ AddRelationNotNullConstraints(Relation rel, List *constraints,
> > conname = other->name;
> >
> > inhcount++;
> > +
> > + /*
> > + * if a column inherit multiple not-null constraints, the
> > + * enforced status should the same.
> > + */
> > + if (other->is_enforced != cooked->is_enforced)
> > + ereport(ERROR,
> > + errcode(ERRCODE_DATATYPE_MISMATCH),
> > + errmsg("cannot define not-null constraint on column \"%s\"", conname),
> > + errdetail("The column inherited not-null constraints have conflict ENFORCED status."));
> > old_notnulls = list_delete_nth_cell(old_notnulls, restpos);
> > }
> > else
>
> Hmmm, are you sure about this? I think if a table has two parents, one
> with enforced and the other with not enforced constraint, then it's okay
> to get them combined resulting in one enforced constraint.
>

changed accordingly.
When a column can inherit multiple not-null constraints. If one is not enforced,
another one is enforced then we will install an enforced one.

> > @@ -777,6 +778,18 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
> > errhint("You might need to validate it using %s.",
> > "ALTER TABLE ... VALIDATE CONSTRAINT"));
> >
> > + /*
> > + * If the ENFORCED status we're asked for doesn't match what the
> > + * existing constraint has, throw an error.
> > + */
> > + if (is_enforced != conform->conenforced)
> > + ereport(ERROR,
> > + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> > + errmsg("cannot change ENFORCED status of NOT NULL constraint \"%s\" on relation \"%s\"",
> > + NameStr(conform->conname), get_rel_name(relid)),
> > + errhint("You might need to drop the existing not enforced constraint using %s.",
> > + "ALTER TABLE ... DROP CONSTRAINT"));
>
> I think the hint here should suggest to make the existing constraint as
> enforced, rather than drop it.
>

The hint also changed.

+ /*
+ * If the ENFORCED status we're asked for doesn't match what the
+ * existing constraint has, throw an error.
+ */
+ if (is_enforced != conform->conenforced)
+ {
+ if (is_enforced)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot change not enforced NOT NULL constraint
\"%s\" on relation \"%s\" to enforced",
+ NameStr(conform->conname), get_rel_name(relid)),
+ errhint("You might need to ensure the existing constraint
is enforced."));
+ else
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot change enforced NOT NULL constraint \"%s\"
on relation \"%s\" to not enforced",
+ NameStr(conform->conname), get_rel_name(relid)),
+ errhint("You might need to ensure the existing constraint
is not enforced."));
+ }

>
> > + else if (notenforced)
> > + {
> > + /*
> > + * We can't use ATExecSetNotNull here because it adds an enforced
> > + * not-null constraint, but here we only want a non-enforced one.
> > + */
>
> Umm, wouldn't it make more sense to modify ATExecSetNotNull() so that it
> does what we want? This seems hackish.
>

modified ATExecSetNotNull for ATExecAlterConstrInheritability usage.
now ATExecSetNotNull is

ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
bool recurse, bool recursing, bool
is_enforced, LOCKMODE lockmode)

new patch attached with the pg_dump TAP tests.
currently NOT VALID NOT NULL dumped
constraint separately, NOT NULL NOT ENFORCED constraints can also be dumped
separately.

CREATE TABLE tx3 (x int not null not enforced);

can be dumped as:

CREATE TABLE public.tx3 (x integer);
ALTER TABLE public.tx3 ADD CONSTRAINT tx3_x_not_null NOT NULL x NOT ENFORCED;
---------------
note: currently not enforced check constraint is dumped separately.
CREATE TABLE tx2 (x int check (x > 1) not enforced);
will be dumped as

CREATE TABLE public.tx2 (x integer);
ALTER TABLE public.tx2
ADD CONSTRAINT tx2_x_check CHECK ((x > 1)) NOT ENFORCED;