Re: NOT NULL NOT ENFORCED - Mailing list pgsql-hackers

On Thu, Sep 4, 2025 at 8:00 PM Álvaro Herrera <alvherre@kurilemu.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
conflictENFORCED 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;