public inbox for [email protected]
help / color / mirror / Atom feedFrom: Álvaro Herrera <[email protected]>
To: Srinath Reddy Sadipiralla <[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: Mon, 2 Feb 2026 20:22:17 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFC+b6oVFELdux6b71LSs=_SJHT-O4MODV40VDy3aOYVR3YkQg@mail.gmail.com>
On 2026-Feb-01, Srinath Reddy Sadipiralla wrote:
> 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.
Okay, it should be all good then. I noticed that some of the changes in
the patch were unnecessary; I had added them transiently to cover the
inheritance case while investigating, but since the real fix only
affects directly specified constraints and doesn't touch inherited ones,
we can remove them. In particular this reverts the unpleasant change
that was going to occur for inherited constraints, which was quite bulky
in the regression tests.
I also reworded the message to be closer to our guidelines and to other
nearby messages, and expanded the code comment that described why we're
doing this check.
Here's the patch in v3, which I intend to push tomorrow morning to both
18 and master. (It backpatches cleanly). For 18 it will mean an ABI
break due to the change to AdjustNotNullInheritance()'s signature,
requiring a touch to .abi-compliance-history as well, but that comes
later.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Attachments:
[text/x-diff] v3-0001-Reject-ADD-CONSTRAINT-NOT-NULL-if-name-mismatches.patch (7.0K, 2-v3-0001-Reject-ADD-CONSTRAINT-NOT-NULL-if-name-mismatches.patch)
download | inline diff:
From 00409a1fd352b2a516d97c21addd64547850cd89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]>
Date: Mon, 2 Feb 2026 20:19:25 +0100
Subject: [PATCH v3] Reject ADD CONSTRAINT NOT NULL if name mismatches existing
constraint
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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.
Reported-by: yanliang lei <[email protected]>
Co-authored-by: Álvaro Herrera <[email protected]>
Co-authored-bu: Srinath Reddy Sadipiralla <[email protected]>
Backpatch-through: 18
Discussion: https://postgr.es/m/19351-8f1c523ead498545%40postgresql.org
---
src/backend/catalog/heap.c | 1 +
src/backend/catalog/pg_constraint.c | 21 +++++++++++++++++++--
src/include/catalog/pg_constraint.h | 2 +-
src/test/regress/expected/constraints.out | 6 +++++-
src/test/regress/sql/constraints.sql | 4 +++-
5 files changed, 29 insertions(+), 5 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..b12765ae691 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 *new_conname,
bool is_local, bool is_no_inherit, bool is_notvalid)
{
HeapTuple tup;
@@ -777,6 +778,22 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum,
errhint("You might need to validate it using %s.",
"ALTER TABLE ... VALIDATE CONSTRAINT"));
+ /*
+ * If, for a new constraint that is being defined locally (i.e., not
+ * being passed down via inheritance), a name was specified, then
+ * verify that the existing constraint has the same name. Otherwise
+ * throw an error. Names of inherited constraints are ignored because
+ * they are not directly user-specified, so matching is not important.
+ */
+ if (is_local && new_conname &&
+ strcmp(new_conname, NameStr(conform->conname)) != 0)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot create not-null constraint \"%s\" on column \"%s\" of table \"%s\"",
+ new_conname, get_attname(relid, attnum, false), get_rel_name(relid)),
+ 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/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 05933cd9741..d5661b5bdff 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 *new_conname,
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..ebc892a2a42 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -846,8 +846,12 @@ CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL NOT NULL);
Not-null constraints:
"notnull_tbl1_a_not_null" NOT NULL "a"
--- no-op
+-- specifying an existing constraint is a no-op
+ALTER TABLE notnull_tbl1 ADD CONSTRAINT notnull_tbl1_a_not_null NOT NULL a;
+-- but using a different constraint name is not allowed
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn NOT NULL a;
+ERROR: cannot create not-null constraint "nn" on column "a" of table "notnull_tbl1"
+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
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 733a1dbccfe..1e9989698b6 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -623,7 +623,9 @@ DROP TABLE deferred_excl;
-- verify constraints created for NOT NULL clauses
CREATE TABLE notnull_tbl1 (a INTEGER NOT NULL NOT NULL);
\d+ notnull_tbl1
--- no-op
+-- specifying an existing constraint is a no-op
+ALTER TABLE notnull_tbl1 ADD CONSTRAINT notnull_tbl1_a_not_null NOT NULL a;
+-- but using a different constraint name is not allowed
ALTER TABLE notnull_tbl1 ADD CONSTRAINT nn NOT NULL a;
\d+ notnull_tbl1
-- duplicate name
--
2.47.3
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: <[email protected]>
* 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