Обсуждение: NOT NULL NOT ENFORCED
hi. attached patch will remove sql_features.txt item F492: "Optional table constraint enforcement" Comment: "except not-null constraints". see [1] main points about NOT NULL NOT ENFORCED * one column can have at most one NOT-NULL constraint, regardless constraints property (not enforced or enforced) * if column already have not enforced not-null constraint then: ALTER TABLE ALTER COLUMN SET NOT NULL: error out, can not validate not enforced not-null constraint ALTER TABLE ADD NOT NULL: error out, can not add another not-null constraint, one column can only have one. not null in partitioned table: * If the partitioned table has an enforced not-null constraint, its partitions cannot have not enforced. * If the partitioned table has a NOT ENFORCED not-null constraint, its partitions may have either ENFORCED or NOT ENFORCED not-null constraints, but the constraint itself is still required. not null in table inheritance: OK: parent is not enforced, while child is enforced NOT OK: parent is enforced, while child is not enforced If a column inherits from multiple tables and the ancestor tables have conflicting ENFORCED statuses, raise an error. I have written extensive tests to cover the corner case, the tests may be overwhelming. [1]: https://www.postgresql.org/docs/devel/features-sql-standard.html
Вложения
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.
> @@ -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.
> @@ -9937,9 +9962,9 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
> * If adding a valid not-null constraint, set the pg_attribute flag
> * and tell phase 3 to verify existing rows, if needed. For an
> * invalid constraint, just set attnotnull, without queueing
> - * verification.
> + * verification. For not enforced not-null, no need set attnotnull.
> */
> - if (constr->contype == CONSTR_NOTNULL)
> + if (constr->contype == CONSTR_NOTNULL && ccon->is_enforced)
> set_attnotnull(wqueue, rel, ccon->attnum,
> !constr->skip_validation,
> !constr->skip_validation);
Didn't we decide that attnotnull meant whether a constraint exists at
all, without making a judgement on whether it's enforced or valid? The
important change should be in CheckNNConstraintFetch() which should
determine attnullability in a way that allows executor know whether the
column is nullable or not. I admit we might want to handle this
differently for unenforced constraints, but we should discuss that and
make sure that's what we want.
> + 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.
> @@ -1272,33 +1294,41 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
> * Reproduce not-null constraints, if any, by copying them. We do this
> * regardless of options given.
> */
> - if (tupleDesc->constr && tupleDesc->constr->has_not_null)
> - {
> - List *lst;
> + lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
> + true);
> + cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
>
> - lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
> - true);
> + /*
> + * When creating a new relation, marking the enforced not-null constraint as
> + * not valid doesn't make sense, so we treat it as valid.
> + */
> + foreach_node(Constraint, nnconstr, lst)
> + {
> + if (nnconstr->is_enforced)
> + {
> + nnconstr->skip_validation = false;
> + nnconstr->initially_valid = true;
> + }
> + }
Hmmm, this bit here (making constraints as valid if they're not valid in
the source table) looks like a fix for the existing code. I think it
should be a separate patch, perhaps back-patchable to 18. Or maybe I'm
missing something ...?
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Digital and video cameras have this adjustment and film cameras don't for the
same reason dogs and cats lick themselves: because they can." (Ken Rockwell)
On Thu, Sep 4, 2025 at 8:00 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> > @@ -1272,33 +1294,41 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
> > * Reproduce not-null constraints, if any, by copying them. We do this
> > * regardless of options given.
> > */
> > - if (tupleDesc->constr && tupleDesc->constr->has_not_null)
> > - {
> > - List *lst;
> > + lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
> > + true);
> > + cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
> >
> > - lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
> > - true);
>
> > + /*
> > + * When creating a new relation, marking the enforced not-null constraint as
> > + * not valid doesn't make sense, so we treat it as valid.
> > + */
> > + foreach_node(Constraint, nnconstr, lst)
> > + {
> > + if (nnconstr->is_enforced)
> > + {
> > + nnconstr->skip_validation = false;
> > + nnconstr->initially_valid = true;
> > + }
> > + }
>
> Hmmm, this bit here (making constraints as valid if they're not valid in
> the source table) looks like a fix for the existing code. I think it
> should be a separate patch, perhaps back-patchable to 18. Or maybe I'm
> missing something ...?
>
it's indeed a bug, which was introduced
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/parser/parse_utilcmd.c?id=ca87c415e2fccf81cec6fd45698dde9fae0ab570
attached is the fix, also added a test on create_table_like.sql
Вложения
On 2025-Sep-04, Álvaro Herrera wrote:
> On 2025-Sep-04, jian he wrote:
> > + 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.
BTW while you're at that, it might make sense to allow commands like
ALTER TABLE foo ALTER col1 SET NOT NULL NOT VALID
ALTER TABLE foo ALTER col1 SET NOT NULL NOT ENFORCED
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"La verdad no siempre es bonita, pero el hambre de ella sí"
On Thu, Sep 4, 2025 at 8:00 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> > @@ -9937,9 +9962,9 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
> > * If adding a valid not-null constraint, set the pg_attribute flag
> > * and tell phase 3 to verify existing rows, if needed. For an
> > * invalid constraint, just set attnotnull, without queueing
> > - * verification.
> > + * verification. For not enforced not-null, no need set attnotnull.
> > */
> > - if (constr->contype == CONSTR_NOTNULL)
> > + if (constr->contype == CONSTR_NOTNULL && ccon->is_enforced)
> > set_attnotnull(wqueue, rel, ccon->attnum,
> > !constr->skip_validation,
> > !constr->skip_validation);
>
> Didn't we decide that attnotnull meant whether a constraint exists at
> all, without making a judgement on whether it's enforced or valid? The
> important change should be in CheckNNConstraintFetch() which should
> determine attnullability in a way that allows executor know whether the
> column is nullable or not. I admit we might want to handle this
> differently for unenforced constraints, but we should discuss that and
> make sure that's what we want.
>
In CheckNNConstraintFetch, I changed it to
"""
if (conform->contype == CONSTRAINT_NOTNULL)
{
if (!conform->convalidated && conform->conenforced)
{
AttrNumber attnum;
attnum = extractNotNullColumn(htup);
Assert(relation->rd_att->compact_attrs[attnum -
1].attnullability ==
ATTNULLABLE_UNKNOWN);
relation->rd_att->compact_attrs[attnum - 1].attnullability =
ATTNULLABLE_INVALID;
}
continue;
}
"""
set pg_attribute.attnotnull to true for not-valid not-null is still
useful for INSERT/UPDATE.
set pg_attribute.attnotnull to true for not-enforced not-null
constraints doesn't have real benefits, IMHO.
If we let pg_attribute.attnotnull to true for not-enforced not-null,
then do we need to change the definition of
TupleConstr->has_not_null?
On 2025-Sep-08, jian he wrote: > set pg_attribute.attnotnull to true for not-valid not-null is still > useful for INSERT/UPDATE. > set pg_attribute.attnotnull to true for not-enforced not-null > constraints doesn't have real benefits, IMHO. Yeah, you might be right about this actually. What we wanted attnotnull to be set for, as I recall, was so that generic apps could alert the user that an insert that tries to put a NULL value in that column would not work, without having to actually execute it. With a non-enforced -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "The important things in the world are problems with society that we don't understand at all. The machines will become more complicated but they won't be more complicated than the societies that run them." (Freeman Dyson)
On 04.09.25 17:20, jian he wrote:
> On Thu, Sep 4, 2025 at 8:00 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>>> @@ -1272,33 +1294,41 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
>>> * Reproduce not-null constraints, if any, by copying them. We do this
>>> * regardless of options given.
>>> */
>>> - if (tupleDesc->constr && tupleDesc->constr->has_not_null)
>>> - {
>>> - List *lst;
>>> + lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
>>> + true);
>>> + cxt->nnconstraints = list_concat(cxt->nnconstraints, lst);
>>>
>>> - lst = RelationGetNotNullConstraints(RelationGetRelid(relation), false,
>>> - true);
>>
>>> + /*
>>> + * When creating a new relation, marking the enforced not-null constraint as
>>> + * not valid doesn't make sense, so we treat it as valid.
>>> + */
>>> + foreach_node(Constraint, nnconstr, lst)
>>> + {
>>> + if (nnconstr->is_enforced)
>>> + {
>>> + nnconstr->skip_validation = false;
>>> + nnconstr->initially_valid = true;
>>> + }
>>> + }
>>
>> Hmmm, this bit here (making constraints as valid if they're not valid in
>> the source table) looks like a fix for the existing code. I think it
>> should be a separate patch, perhaps back-patchable to 18. Or maybe I'm
>> missing something ...?
>>
>
> it's indeed a bug, which was introduced
>
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/parser/parse_utilcmd.c?id=ca87c415e2fccf81cec6fd45698dde9fae0ab570
>
> attached is the fix, also added a test on create_table_like.sql
I have committed this fix.
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;
Вложения
On 2025-Sep-24, jian he wrote: > 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. Hmm, I wonder what's the reason for this. Seems quite useless. Why wouldn't we dump unenforced constraint together with the table? The case is different from invalid constraints, which have to be created after data is loaded. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/