public inbox for [email protected]help / color / mirror / Atom feed
Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 7+ messages / 2 participants [nested] [flat]
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. @ 2025-12-13 12:44 Álvaro Herrera <[email protected]> 2025-12-14 17:00 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 11:45 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Álvaro Herrera @ 2025-12-13 12:44 UTC (permalink / raw) To: yanliang lei <[email protected]>; +Cc: Srinath Reddy Sadipiralla <[email protected]>; [email protected] 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) Attachments: [text/x-diff] fix.patch (9.3K, 2-fix.patch) download | inline diff: 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> @ 2025-12-14 17:00 ` Srinath Reddy Sadipiralla <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Srinath Reddy Sadipiralla @ 2025-12-14 17:00 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: yanliang lei <[email protected]>; [email protected] Hi Álvaro, On Sat, Dec 13, 2025 at 6:14 PM Álvaro Herrera <[email protected]> wrote: > > Yeah, that's fair -- we discussed this kind of behavior during > development and I was unsure about being strict about it, so avoided it. > > +1 , thanks for the patch. This is mostly straightforward to fix, as attached (though the error > message needs work), I have reviewed the patch except the below things and LGTM until now. > 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. > I need to look further into these parts,and will update once I have something solid. -- Thanks, Srinath Reddy Sadipiralla EDB: https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> @ 2026-01-26 11:45 ` Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 14:33 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Srinath Reddy Sadipiralla @ 2026-01-26 11:45 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: yanliang lei <[email protected]>; [email protected] Hi Álvaro, On Sat, Dec 13, 2025 at 6:14 PM Álvaro Herrera <[email protected]> wrote: > 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. > i looked into this, the reason for these failures was when the given name for a constraint for a parent table propagates to the child table because of inheritance the name conflicts and throws "mismatching constraint name" error we added, let me show an example, postgres=# create table test1(col1 int); CREATE TABLE postgres=# create table test2(col1 int not null); CREATE TABLE postgres=# create table child12() inherits ( test1,test2); NOTICE: merging multiple inherited definitions of column "col1" CREATE TABLE postgres=# \d+ child12 Table "public.child12" Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- col1 | integer | | not null | | plain | | | Not-null constraints: "test2_col1_not_null" NOT NULL "col1" (inherited) Inherits: test1, test2 Access method: heap postgres=# alter table test1 add constraint nn not null col1 not valid; ERROR: mismatching constraint name "nn" DETAIL: A not-null constraint named "test2_col1_not_null" already exists for this column. 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. /* * 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))); also checking how other constraints handle this case like CHECK and found it just appends to existing constraint postgres=# \d+ child34 Table "public.child34" Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- a | integer | | | | plain | | | Check constraints: "c" CHECK (a > 1) "d" CHECK (a > 1) Inherits: test3, test4 Access method: heap but I don't think it makes sense for NOT NULL, thoughts? -- Thanks, Srinath Reddy Sadipiralla EDB: https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2026-01-26 11:45 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> @ 2026-01-26 14:33 ` Álvaro Herrera <[email protected]> 2026-01-26 15:34 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 2026-02-01 12:19 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Álvaro Herrera @ 2026-01-26 14:33 UTC (permalink / raw) To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: yanliang lei <[email protected]>; [email protected] Hello, On 2026-Jan-26, Srinath Reddy Sadipiralla wrote: > i looked into this, Thank you! > the reason for these failures was when the given name for a constraint > for a parent table propagates to the child table because of > inheritance the name conflicts and throws "mismatching constraint > name" error we added, Right, that's what I saw. > 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 Would you be able to send a patch based on this idea and what I sent earlier? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Las navajas y los monos deben estar siempre distantes" (Germán Poo) ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2026-01-26 11:45 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 14:33 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> @ 2026-01-26 15:34 ` Srinath Reddy Sadipiralla <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Srinath Reddy Sadipiralla @ 2026-01-26 15:34 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: yanliang lei <[email protected]>; [email protected] 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 > Thanks for the pointers, will look into this angle also. Would you be able to send a patch based on this idea and what I sent > earlier? > sure , I will do that. -- Thanks, Srinath Reddy Sadipiralla EDB: https://www.enterprisedb.com/ ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2026-01-26 11:45 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 14:33 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> @ 2026-02-01 12:19 ` Srinath Reddy Sadipiralla <[email protected]> 2026-02-02 19:22 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Srinath Reddy Sadipiralla @ 2026-02-01 12:19 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: yanliang lei <[email protected]>; [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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2026-01-26 11:45 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 14:33 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2026-02-01 12:19 ` Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Srinath Reddy Sadipiralla <[email protected]> @ 2026-02-02 19:22 ` Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Álvaro Herrera @ 2026-02-02 19:22 UTC (permalink / raw) To: Srinath Reddy Sadipiralla <[email protected]>; +Cc: yanliang lei <[email protected]>; [email protected] 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-02-02 19:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-12-13 12:44 Re: Re: Re: BUG #19351: in pg18.1,when not null exists in the table , and add constraint problem. Álvaro Herrera <[email protected]> 2025-12-14 17:00 ` Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 11:45 ` Srinath Reddy Sadipiralla <[email protected]> 2026-01-26 14:33 ` Álvaro Herrera <[email protected]> 2026-01-26 15:34 ` Srinath Reddy Sadipiralla <[email protected]> 2026-02-01 12:19 ` Srinath Reddy Sadipiralla <[email protected]> 2026-02-02 19:22 ` Álvaro Herrera <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox