public inbox for [email protected]
help / color / mirror / Atom feedFrom: Vismay Tiwari <[email protected]>
To: [email protected]
Subject: Re: BUG #19507: Auto-named partition table constraint conflicts
Date: Thu, 9 Jul 2026 14:45:50 +0530
Message-ID: <CALHMmB_q1Sme-7_OvGiZAyJ6eSB8eKkfRuwreZK3rO88Nfk78w@mail.gmail.com> (raw)
Hi,
Thanks for the report, Marko. I could reproduce both cases on current master.
The root of it is that when the constraint name is auto-generated for the
partitioned parent, ChooseConstraintName only checks for a clash within the
parent's own schema. So a partition sitting in another schema that already has
a constraint by that name isn't noticed, and the name then collides once it's
propagated down. A same-schema partition doesn't trip this, because the
namespace-scoped lookup already sees that one and picks a non-conflicting name;
that makes it specific to partitions in other schemas.
The attached patch gathers the constraint names already present on the
descendants and passes them to ChooseConstraintName as names to avoid, so the
generated name comes out unique across the whole hierarchy. I deliberately kept
those names out of the checknames/nnnames lists, since those also drive the
duplicate check for explicitly-named constraints, and folding them in would
break legitimate merges of same-named inherited constraints. It's guarded on
relhassubclass, so there's no extra work for tables without children.
I added a regression test in constraints.sql covering both the SET NOT NULL and
ADD CHECK cases; make check passes.
One thing I didn't chase down: foreign-key names go through a similar
auto-naming path, so they may have the same issue. I haven't tested that, so
I'm flagging it in case it's worth a look.
Regards,
Vismay Tiwari
Attachments:
[application/octet-stream] v1-0001-Avoid-auto-generated-constraint-name-collisions-a.patch (12.6K, ../CALHMmB_q1Sme-7_OvGiZAyJ6eSB8eKkfRuwreZK3rO88Nfk78w@mail.gmail.com/2-v1-0001-Avoid-auto-generated-constraint-name-collisions-a.patch)
download | inline diff:
From 2cdfa364b9409c7bfe2c45abcadf2e0a4b2cae5b Mon Sep 17 00:00:00 2001
From: vismaytiwari <[email protected]>
Date: Thu, 9 Jul 2026 14:19:47 +0530
Subject: [PATCH v1] Avoid auto-generated constraint name collisions across
partitions
When a NOT NULL or CHECK constraint is added to a partitioned table (or
other inheritance parent) without an explicit name, the parent picks a name
with ChooseConstraintName and then propagates that same name down to all of
its children. ChooseConstraintName only checks for a clash within the
parent's own schema, so if a child living in another schema already has a
constraint by the generated name, propagation fails with
ERROR: constraint "..." for relation "..." already exists
even though the user never chose the colliding name. A child in the *same*
schema does not trigger this, because ChooseConstraintName's namespace-scoped
scan already sees it and picks a non-conflicting name; the problem is
specific to descendants in other schemas.
Fix by gathering the constraint names already present on the relation's
descendants and passing them to ChooseConstraintName as additional names to
avoid, so the generated name is unique across the whole hierarchy rather
than only within the parent's schema. The descendant names are fed only to
the name generator; they are deliberately kept out of the checknames/nnnames
lists, which also serve as the duplicate check for explicitly-named
constraints in the same command, so legitimate merges of same-named
inherited constraints keep working.
Reported-by: Marko Grujic
Bug: #19507
Discussion: https://postgr.es/m/[email protected]
---
src/backend/catalog/heap.c | 21 ++++++++
src/backend/catalog/pg_constraint.c | 60 +++++++++++++++++++++++
src/backend/commands/tablecmds.c | 14 +++++-
src/include/catalog/pg_constraint.h | 1 +
src/test/regress/expected/constraints.out | 33 +++++++++++++
src/test/regress/sql/constraints.sql | 27 ++++++++++
6 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8808765..c304704 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2419,6 +2419,7 @@ AddRelationNewConstraints(Relation rel,
int numchecks;
List *checknames;
List *nnnames;
+ List *inherited_names;
Node *expr;
CookedConstraint *cooked;
@@ -2499,6 +2500,22 @@ AddRelationNewConstraints(Relation rel,
numchecks = numoldchecks;
checknames = NIL;
nnnames = NIL;
+
+ /*
+ * If the relation has inheritance children (for example, it is a
+ * partitioned table), a constraint name we generate automatically will be
+ * propagated down to those children, which may live in other schemas.
+ * ChooseConstraintName already avoids names used by same-schema children
+ * (via its namespace-scoped check), but not ones in other schemas, so
+ * gather the constraint names present on descendant tables and pass them
+ * to ChooseConstraintName below. That keeps the generated name unique
+ * across the whole hierarchy rather than just this relation's schema.
+ * Note we must not fold these into checknames/nnnames, which double as the
+ * duplicate check for explicitly-named constraints in this same command.
+ */
+ inherited_names = NIL;
+ if (rel->rd_rel->relhassubclass)
+ inherited_names = GetInheritedConstraintNames(RelationGetRelid(rel));
foreach_node(Constraint, cdef, newConstraints)
{
Oid constrOid;
@@ -2596,6 +2613,8 @@ AddRelationNewConstraints(Relation rel,
colname,
"check",
RelationGetNamespace(rel),
+ inherited_names ?
+ list_concat_copy(checknames, inherited_names) :
checknames);
/* save name for future checks */
@@ -2680,6 +2699,8 @@ AddRelationNewConstraints(Relation rel,
strVal(linitial(cdef->keys)),
"not_null",
RelationGetNamespace(rel),
+ inherited_names ?
+ list_concat_copy(nnnames, inherited_names) :
nnnames);
nnnames = lappend(nnnames, nnname);
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index b12765a..cdff84a 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -25,6 +25,7 @@
#include "catalog/indexing.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_constraint.h"
+#include "catalog/pg_inherits.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
@@ -579,6 +580,65 @@ ChooseConstraintName(const char *name1, const char *name2,
return conname;
}
+/*
+ * Collect the names of the constraints defined on the inheritance children
+ * (including partitions, recursively) of the given relation.
+ *
+ * This is meant for use when automatically generating a constraint name for a
+ * relation whose constraint will be propagated to its children. Such children
+ * may live in other schemas, where ChooseConstraintName's namespace-scoped
+ * uniqueness check would not notice an existing constraint of the same name.
+ * Passing the returned list to ChooseConstraintName (directly, or by seeding
+ * the name lists tracked while several constraints are created at once) makes
+ * the generated name unique across the whole hierarchy, not just our schema.
+ *
+ * No locks are taken on the child relations here (only catalog access is
+ * performed); a caller doing DDL already holds a suitable lock on the
+ * hierarchy. The returned strings are palloc'd in the current memory context.
+ */
+List *
+GetInheritedConstraintNames(Oid relid)
+{
+ List *result = NIL;
+ List *children;
+ ListCell *lc;
+ Relation pg_constraint;
+
+ children = find_all_inheritors(relid, NoLock, NULL);
+
+ pg_constraint = table_open(ConstraintRelationId, AccessShareLock);
+
+ foreach(lc, children)
+ {
+ Oid childrelid = lfirst_oid(lc);
+ SysScanDesc scan;
+ ScanKeyData key;
+ HeapTuple tup;
+
+ /* find_all_inheritors includes the relation itself; skip it */
+ if (childrelid == relid)
+ continue;
+
+ ScanKeyInit(&key,
+ Anum_pg_constraint_conrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(childrelid));
+ scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId,
+ true, NULL, 1, &key);
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup);
+
+ result = lappend(result, pstrdup(NameStr(con->conname)));
+ }
+ systable_endscan(scan);
+ }
+
+ table_close(pg_constraint, AccessShareLock);
+
+ return result;
+}
+
/*
* Find and return a copy of the pg_constraint tuple that implements a
* (possibly not valid) not-null constraint for the given column of the
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb93c3e..fb2e163 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8174,11 +8174,23 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
*/
if (!recursing)
{
+ List *others = NIL;
+
Assert(conName == NULL);
+
+ /*
+ * If this table has inheritance children, the name we choose will be
+ * propagated to them (possibly across schemas), so avoid one that
+ * collides with a constraint already present on a descendant;
+ * ChooseConstraintName by itself only checks this table's schema.
+ */
+ if (rel->rd_rel->relhassubclass)
+ others = GetInheritedConstraintNames(RelationGetRelid(rel));
+
conName = ChooseConstraintName(RelationGetRelationName(rel),
colName, "not_null",
RelationGetNamespace(rel),
- NIL);
+ others);
}
constraint = makeNotNullConstraint(makeString(colName));
diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 1b7fedf..3b44229 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -262,6 +262,7 @@ extern bool ConstraintNameExists(const char *conname, Oid namespaceid);
extern char *ChooseConstraintName(const char *name1, const char *name2,
const char *label, Oid namespaceid,
List *others);
+extern List *GetInheritedConstraintNames(Oid relid);
extern HeapTuple findNotNullConstraintAttnum(Oid relid, AttrNumber attnum);
extern HeapTuple findNotNullConstraint(Oid relid, const char *colname);
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index 83f97f6..a63b5f3 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -1791,6 +1791,39 @@ COMMENT ON CONSTRAINT constr_parent2_a_not_null ON constr_parent2 IS 'this const
COMMENT ON CONSTRAINT constr_parent2_a_not_null ON constr_child2 IS 'this constraint is valid';
DEALLOCATE get_nnconstraint_info;
-- end NOT NULL NOT VALID
+-- An auto-generated constraint name for a partitioned table must not collide
+-- with a constraint that already exists on a partition in another schema: the
+-- parent chooses the name, but ChooseConstraintName only checks for a clash
+-- within the parent's own schema (bug #19507).
+CREATE SCHEMA regress_con_part;
+-- parent SET NOT NULL, when a partition in another schema already has a
+-- constraint by the auto-generated not-null name
+CREATE TABLE con_pt1 (a int) PARTITION BY RANGE (a);
+CREATE TABLE regress_con_part.con_pt1_1 PARTITION OF con_pt1 FOR VALUES FROM (1) TO (10);
+ALTER TABLE regress_con_part.con_pt1_1 ADD CONSTRAINT con_pt1_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE con_pt1 ALTER COLUMN a SET NOT NULL;
+-- parent ADD CHECK, when such a partition already has a constraint by the
+-- auto-generated check name
+CREATE TABLE con_pt2 (a int) PARTITION BY RANGE (a);
+CREATE TABLE regress_con_part.con_pt2_1 PARTITION OF con_pt2 FOR VALUES FROM (1) TO (10);
+ALTER TABLE regress_con_part.con_pt2_1 ADD CONSTRAINT con_pt2_a_check CHECK (a > 100);
+ALTER TABLE con_pt2 ADD CHECK (a > 0);
+-- the parent's auto-generated names were uniquified to avoid the collision
+SELECT conrelid::regclass::text AS partition, conname, contype
+ FROM pg_constraint
+ WHERE conrelid IN ('regress_con_part.con_pt1_1'::regclass,
+ 'regress_con_part.con_pt2_1'::regclass)
+ ORDER BY 1, 2;
+ partition | conname | contype
+----------------------------+---------------------+---------
+ regress_con_part.con_pt1_1 | con_pt1_a_not_null | c
+ regress_con_part.con_pt1_1 | con_pt1_a_not_null1 | n
+ regress_con_part.con_pt2_1 | con_pt2_a_check | c
+ regress_con_part.con_pt2_1 | con_pt2_a_check1 | c
+(4 rows)
+
+DROP TABLE con_pt1, con_pt2;
+DROP SCHEMA regress_con_part;
-- Comments
-- Setup a low-level role to enforce non-superuser checks.
CREATE ROLE regress_constraint_comments;
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 9705962..1aa8397 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -1072,6 +1072,33 @@ DEALLOCATE get_nnconstraint_info;
-- end NOT NULL NOT VALID
+-- An auto-generated constraint name for a partitioned table must not collide
+-- with a constraint that already exists on a partition in another schema: the
+-- parent chooses the name, but ChooseConstraintName only checks for a clash
+-- within the parent's own schema (bug #19507).
+CREATE SCHEMA regress_con_part;
+-- parent SET NOT NULL, when a partition in another schema already has a
+-- constraint by the auto-generated not-null name
+CREATE TABLE con_pt1 (a int) PARTITION BY RANGE (a);
+CREATE TABLE regress_con_part.con_pt1_1 PARTITION OF con_pt1 FOR VALUES FROM (1) TO (10);
+ALTER TABLE regress_con_part.con_pt1_1 ADD CONSTRAINT con_pt1_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE con_pt1 ALTER COLUMN a SET NOT NULL;
+-- parent ADD CHECK, when such a partition already has a constraint by the
+-- auto-generated check name
+CREATE TABLE con_pt2 (a int) PARTITION BY RANGE (a);
+CREATE TABLE regress_con_part.con_pt2_1 PARTITION OF con_pt2 FOR VALUES FROM (1) TO (10);
+ALTER TABLE regress_con_part.con_pt2_1 ADD CONSTRAINT con_pt2_a_check CHECK (a > 100);
+ALTER TABLE con_pt2 ADD CHECK (a > 0);
+-- the parent's auto-generated names were uniquified to avoid the collision
+SELECT conrelid::regclass::text AS partition, conname, contype
+ FROM pg_constraint
+ WHERE conrelid IN ('regress_con_part.con_pt1_1'::regclass,
+ 'regress_con_part.con_pt2_1'::regclass)
+ ORDER BY 1, 2;
+DROP TABLE con_pt1, con_pt2;
+DROP SCHEMA regress_con_part;
+
+
-- Comments
-- Setup a low-level role to enforce non-superuser checks.
CREATE ROLE regress_constraint_comments;
--
2.50.1 (Apple Git-155)
view thread (7+ messages) latest in thread
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]
Subject: Re: BUG #19507: Auto-named partition table constraint conflicts
In-Reply-To: <CALHMmB_q1Sme-7_OvGiZAyJ6eSB8eKkfRuwreZK3rO88Nfk78w@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