Feedback-ID: ie3de48e3:Fastmail
Date: Sat, 13 Dec 2025 13:44:38 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: yanliang lei <msdnchina@163.com>
Cc: Srinath Reddy Sadipiralla <srinath2133@gmail.com>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table
 , and add constraint problem.
Message-ID: <202512130812.375tew7hjksb@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="d5mg3wxbwvtxnnok"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <6ec30f7.19f1.19b15ec1942.Coremail.msdnchina@163.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202512130812.375tew7hjksb%40alvherre.pgsql>
Precedence: bulk


--d5mg3wxbwvtxnnok
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Dec-13, yanliang lei wrote:

> Step 2: The execution of 'alter table test_null_20251210 add constraint xyzxyz not null c1' did not return any errors.
> 
> after the execution of Step 2 , the executing user of the SQL
> statement "alter table test_null_20251210 add constraint xyzxyz not
> null c1" in the Step 2 will assume (because there was no error prompt
> in the  Step 2   ) that the not null constraint has been successfully
> added to column c1, and the name of the constraint is xyzxyz.

>  ----However, in reality, based on the execution results of the SQL
>  database, the Step 2 did not actually succeed.

Yeah, that's fair -- we discussed this kind of behavior during
development and I was unsure about being strict about it, so avoided it.

This is mostly straightforward to fix, as attached (though the error
message needs work), but I hit a snag with multiple inheritance -- if
you apply this patch, you'll see failures in the pg_dump and pg_upgrade
tests.  I don't have any ideas to fix this right now, but I'll keep
thinking about it.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La experiencia nos dice que el hombre peló millones de veces las patatas,
pero era forzoso admitir la posibilidad de que en un caso entre millones,
las patatas pelarían al hombre" (Ijon Tichy)

--d5mg3wxbwvtxnnok
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename="fix.patch"

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 265cc3e5fbf..0e86445d58e 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 43b4507d86e..a4fa5a995fc 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 (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 1c9ef53be20..9fac531ebc7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9958,7 +9958,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;
 
 		/*
@@ -9975,8 +9975,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 4afceb5c692..58e94a19ea5 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
 

--d5mg3wxbwvtxnnok--