public inbox for [email protected]
help / color / mirror / Atom feedFrom: Srinath Reddy Sadipiralla <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: yanliang lei <[email protected]>
Cc: [email protected]
Subject: Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem.
Date: Sun, 1 Feb 2026 17:49:07 +0530
Message-ID: <CAFC+b6oVFELdux6b71LSs=_SJHT-O4MODV40VDy3aOYVR3YkQg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAFC+b6qKTes6Uz5gfjq=N0=AcOi5RD=JnRqbU-WnhN2dC21YDw@mail.gmail.com>
<[email protected]>
Hi Álvaro
On Mon, Jan 26, 2026 at 8:03 PM Álvaro Herrera <[email protected]> wrote:
>
>
> > I think we can fix this by throwing an error only if this constraint
> > was added directly to the table and not through
> > inheritance/propagation from the parent, we can do this using the
> > "is_local" flag, i have checked and all tests passed.
>
> Hmm, I'm not opposed to this; does it change any other behavior? I
> think it's important to see whether there are other corner cases that
> would react to this behavior change. For example, what would happen if
> two existing parents have a not-null constraint on the same column? Is
> there a change for combined LIKE and regular inheritance? I think we
> should have reasonable reactions to each of those scenarios:
>
> create table parent (a int not null);
> create table parent2 (a int not null);
>
> create table child1 () inherits (parent, parent2);
> create table child2 () inherits (parent2, parent);
> create table child3 (not null a) inherits (parent2, parent);
>
> create table child4 (like parent) inherits (parent2);
> -- and so on as your imagination allows
>
as you have suggested i have looked whether it effects the other behaviour
,during table creation with not null constraints i observed that flow
doesn't
touch the AdjustNotNullInheritance where we added the error message,
When running CREATE TABLE, the standard NOT NULL merging logic is
handled by DefineRelation -> AddRelationNotNullConstraints. This function
explicitly handles the "Constraint Selection" logic (prioritizing the
Child's
constraint if present, otherwise defaulting to the 1st parent's constraint),
please correct me if I totally understood your concerns wrong here.
> Would you be able to send a patch based on this idea and what I sent
> earlier?
>
I've attached the updated patch.
--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/
Attachments:
[application/octet-stream] v2-0001-Reject-ADD-CONSTRAINT-NOT-NULL-if-name-mismatches-ex.patch (11.1K, 3-v2-0001-Reject-ADD-CONSTRAINT-NOT-NULL-if-name-mismatches-ex.patch)
download | inline diff:
From caa6448c82737d98f581f064f94fef107a749f86 Mon Sep 17 00:00:00 2001
From: srinathv2 <[email protected]>
Date: Sun, 1 Feb 2026 17:33:02 +0530
Subject: [PATCH 1/1] Reject ADD CONSTRAINT NOT NULL if name mismatches
existing constraint
When using ALTER TABLE ... ADD CONSTRAINT to add a NOT NULL constraint
with an explicit name, we should ensure that if the column is already
marked NOT NULL, the provided name matches the existing constraint name.
Failing to do so could lead to confusion regarding which constraint
object actually enforces the rule.
This patch adds a check to throw an error ("mismatching constraint name")
if a user tries to add a named NOT NULL constraint to a column that
already has one with a different name.
However, an exception is made for inheritance recursion. When adding a
constraint to a parent table, the operation propagates to child tables.
A child table may already possess a NOT NULL constraint inherited from
a different parent (and thus bearing a different name). In this scenario,
strictly enforcing the name match would cause the operation on the parent
to fail. Therefore, the name check is restricted to cases where the
constraint is being added locally (is_local is true). If the constraint
is being added via inheritance, we merge it silently with the existing
definition.
Discussion: https://www.postgresql.org/message-id/flat/19351-8f1c523ead498545%40postgresql.org
---
src/backend/catalog/heap.c | 1 +
src/backend/catalog/pg_constraint.c | 17 +++++++++++++++--
src/backend/commands/tablecmds.c | 8 +++++---
src/include/catalog/pg_constraint.h | 2 +-
src/test/regress/expected/constraints.out | 15 ++++++++++-----
src/test/regress/expected/foreign_data.out | 6 +++---
6 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 606434823cf..a6ed9849e77 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2635,6 +2635,7 @@ AddRelationNewConstraints(Relation rel,
* requested validity.
*/
if (AdjustNotNullInheritance(RelationGetRelid(rel), colnum,
+ cdef->conname,
is_local, cdef->is_no_inherit,
cdef->skip_validation))
continue;
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index cbbcf166e45..bced9c1d816 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -731,14 +731,15 @@ extractNotNullColumn(HeapTuple constrTup)
* If a constraint exists but the connoinherit flag is not what the caller
* wants, throw an error about the incompatibility. If the desired
* constraint is valid but the existing constraint is not valid, also
- * throw an error about that (the opposite case is acceptable).
+ * throw an error about that (the opposite case is acceptable). If
+ * the proposed constraint has a different name, also throw an error.
*
* If everything checks out, we adjust conislocal/coninhcount and return
* true. If is_local is true we flip conislocal true, or do nothing if
* it's already true; otherwise we increment coninhcount by 1.
*/
bool
-AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
+AdjustNotNullInheritance(Oid relid, AttrNumber attnum, const char *name,
bool is_local, bool is_no_inherit, bool is_notvalid)
{
HeapTuple tup;
@@ -777,6 +778,18 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
errhint("You might need to validate it using %s.",
"ALTER TABLE ... VALIDATE CONSTRAINT"));
+ /*
+ * Throw an error if the proposed constraint name doesn't match the
+ * existing one.
+ */
+ if (is_local && name &&
+ strcmp(name, NameStr(conform->conname)) != 0)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("mismatching constraint name \"%s\"", name),
+ errdetail("A not-null constraint named \"%s\" already exists for this column.",
+ NameStr(conform->conname)));
+
if (!is_local)
{
if (pg_add_s16_overflow(conform->coninhcount, 1,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f976c0e5c7e..5302846d2a3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9996,7 +9996,7 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
}
/* Save the actually assigned name if it was defaulted */
- if (constr->conname == NULL)
+ if (ccon->contype == CONSTR_CHECK && constr->conname == NULL)
constr->conname = ccon->name;
/*
@@ -10013,8 +10013,10 @@ ATAddCheckNNConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
ObjectAddressSet(address, ConstraintRelationId, ccon->conoid);
}
- /* At this point we must have a locked-down name to use */
- Assert(newcons == NIL || constr->conname != NULL);
+ /* At this point, CHECK constraints must have a locked-down name to use */
+ Assert(newcons == NIL ||
+ constr->contype != CONSTR_CHECK ||
+ constr->conname != NULL);
/* Advance command counter in case same table is visited multiple times */
CommandCounterIncrement();
diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 05933cd9741..f16230e30f1 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -263,7 +263,7 @@ 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 AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
+extern bool AdjustNotNullInheritance(Oid relid, AttrNumber attnum, const char *name,
bool is_local, bool is_no_inherit, bool is_notvalid);
extern List *RelationGetNotNullConstraints(Oid relid, bool cooked,
bool include_noinh);
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 1bbf59cca02..a77f71bb0a8 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -848,6 +848,8 @@ Not-null constraints:
-- no-op
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn NOT NULL a;
+ERROR: mismatching constraint name "nn"
+DETAIL: A not-null constraint named "notnull_tbl1_a_not_null" already exists for this column.
\d+ notnull_tbl1
Table "public.notnull_tbl1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
@@ -1115,7 +1117,7 @@ Child tables: cnn_pk_child
a | integer | | | | plain | |
b | integer | | not null | | plain | |
Not-null constraints:
- "cnn_pk_b_not_null" NOT NULL "b" (inherited)
+ "cnn_pk_child_b_not_null" NOT NULL "b" (inherited)
Inherits: cnn_pk
ALTER TABLE cnn_pk DROP CONSTRAINT cnn_primarykey;
@@ -1135,7 +1137,7 @@ Child tables: cnn_pk_child
a | integer | | | | plain | |
b | integer | | not null | | plain | |
Not-null constraints:
- "cnn_pk_b_not_null" NOT NULL "b" (inherited)
+ "cnn_pk_child_b_not_null" NOT NULL "b" (inherited)
Inherits: cnn_pk
DROP TABLE cnn_pk, cnn_pk_child;
@@ -1208,7 +1210,7 @@ Child tables: cnn_pk_child
a | integer | | | | plain | |
b | integer | | not null | | plain | |
Not-null constraints:
- "cnn_pk_b_not_null" NOT NULL "b" (inherited)
+ "cnn_pk_child_b_not_null" NOT NULL "b" (inherited)
Inherits: cnn_pk
DROP TABLE cnn_pk, cnn_pk_child;
@@ -1640,10 +1642,11 @@ create table constr_child (a int) inherits (constr_parent);
NOTICE: merging column "a" with inherited definition
alter table constr_parent add not null a not valid;
alter table constr_child validate constraint constr_parent_a_not_null;
+ERROR: constraint "constr_parent_a_not_null" of relation "constr_child" does not exist
EXECUTE get_nnconstraint_info('{constr_parent, constr_child}');
tabname | conname | convalidated | conislocal | coninhcount
---------------+--------------------------+--------------+------------+-------------
- constr_child | constr_parent_a_not_null | t | f | 1
+ constr_child | constr_child_a_not_null | f | f | 1
constr_parent | constr_parent_a_not_null | f | t | 0
(2 rows)
@@ -1651,10 +1654,11 @@ create table constr_parent2 (a int);
create table constr_child2 () inherits (constr_parent2);
alter table constr_parent2 add not null a not valid;
alter table constr_child2 validate constraint constr_parent2_a_not_null;
+ERROR: constraint "constr_parent2_a_not_null" of relation "constr_child2" does not exist
EXECUTE get_nnconstraint_info('{constr_parent2, constr_child2}');
tabname | conname | convalidated | conislocal | coninhcount
----------------+---------------------------+--------------+------------+-------------
- constr_child2 | constr_parent2_a_not_null | t | f | 1
+ constr_child2 | constr_child2_a_not_null | f | f | 1
constr_parent2 | constr_parent2_a_not_null | f | t | 0
(2 rows)
@@ -1670,6 +1674,7 @@ EXECUTE get_nnconstraint_info('{constr_parent3, constr_child3}');
COMMENT ON CONSTRAINT constr_parent2_a_not_null ON constr_parent2 IS 'this constraint is invalid';
COMMENT ON CONSTRAINT constr_parent2_a_not_null ON constr_child2 IS 'this constraint is valid';
+ERROR: constraint "constr_parent2_a_not_null" for table "constr_child2" does not exist
DEALLOCATE get_nnconstraint_info;
-- end NOT NULL NOT VALID
-- Comments
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index cce49e509ab..68e0f33a65b 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1569,7 +1569,7 @@ Child tables: ft2, FOREIGN
c8 | integer | | | | | plain | |
Not-null constraints:
"ft2_c1_not_null" NOT NULL "c1" (local, inherited)
- "fd_pt1_c7_not_null" NOT NULL "c7" (inherited)
+ "ft2_c7_not_null" NOT NULL "c7" (inherited)
Server: s0
FDW options: (delimiter ',', quote '"', "be quoted" 'value')
Inherits: fd_pt1
@@ -1590,7 +1590,7 @@ Child tables: ct3,
c8 | integer | | | | plain | |
Not-null constraints:
"ft2_c1_not_null" NOT NULL "c1" (inherited)
- "fd_pt1_c7_not_null" NOT NULL "c7" (inherited)
+ "ct3_c7_not_null" NOT NULL "c7" (inherited)
Inherits: ft2
\d+ ft3
@@ -1607,7 +1607,7 @@ Inherits: ft2
c8 | integer | | | | | plain | |
Not-null constraints:
"ft3_c1_not_null" NOT NULL "c1" (local, inherited)
- "fd_pt1_c7_not_null" NOT NULL "c7" (inherited)
+ "ft3_c7_not_null" NOT NULL "c7" (inherited)
Server: s0
Inherits: ft2
--
2.43.0
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem.
In-Reply-To: <CAFC+b6oVFELdux6b71LSs=_SJHT-O4MODV40VDy3aOYVR3YkQg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox