[PATCH v1] [BUG #19507] Prevent constraint name conflicts in partition trees spanning multiple schemas

ChooseConstraintName picks a constraint name unique within a given
relation's namespace. When it is invoked for a partition root, the
chosen name is not guaranteed to be unique for any partition in a
different schema.

This patch introduces ChooseConstraintNameForRelation(Oid relid, ...),
which wraps ChooseConstraintName, but also excludes all pre-existing
constraints across the entire partition hierarchy. When the relation
has no descendants their behavior is identical.

In addition several test cases are added which exercise the patterns
vulnerable to this bug. Namely, invoking ALTER COLUMN ... SET NOT NULL,
ADD NOT NULL, and ADD CHECK on a partitioned root, when a partition
in another schema has a pre-existing constraint.

Bug: #19507
---
src/backend/catalog/heap.c | 18 ++---
src/backend/catalog/pg_constraint.c | 81 +++++++++++++++++++
src/backend/commands/tablecmds.c | 7 +-
src/include/catalog/pg_constraint.h | 2 +
src/test/regress/expected/constraints.out | 95 +++++++++++++++++++++++
src/test/regress/sql/constraints.sql | 65 ++++++++++++++++
6 files changed, 254 insertions(+), 14 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 88087654de9..af29d7f8f64 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2592,11 +2592,10 @@ AddRelationNewConstraints(Relation rel,
else
colname = NULL;

- ccname = ChooseConstraintName(RelationGetRelationName(rel),
- colname,
- "check",
- RelationGetNamespace(rel),
- checknames);
+ ccname = ChooseConstraintNameForRelation(RelationGetRelid(rel),
+ colname,
+ "check",
+ checknames);

/* save name for future checks */
checknames = lappend(checknames, ccname);
@@ -2676,11 +2675,10 @@ AddRelationNewConstraints(Relation rel,
nnname = cdef->conname;
}
else
- nnname = ChooseConstraintName(RelationGetRelationName(rel),
- strVal(linitial(cdef->keys)),
- "not_null",
- RelationGetNamespace(rel),
- nnnames);
+ nnname = ChooseConstraintNameForRelation(RelationGetRelid(rel),
+ strVal(linitial(cdef->keys)),
+ "not_null",
+ nnnames);
nnnames = lappend(nnnames, nnname);

constrOid =
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index b12765ae691..ba9cd038723 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,86 @@ ChooseConstraintName(const char *name1, const char *name2,
return conname;
}

+/*
+ * Choose a constraint name for a relation.
+ *
+ * Like ChooseConstraintName, but uses the relation's OID to derive its
+ * name and namespace.
+ *
+ * In addition, it rules out names already in use by any constraint on
+ * any descendant of the relation (e.g. partitions).
+ *
+ * This is needed because constraint-name uniqueness is enforced
+ * per-relation (conrelid, conname), while ChooseConstraintName's own
+ * uniqueness scan is per-namespace (connamespace, conname). When a
+ * partition tree spans multiple schemas, a name that looks free at the
+ * root's namespace can still collide with an unrelated constraint on a
+ * leaf in a different schema, causing the recursive descent into that
+ * leaf to fail. Gathering descendant constraint names into the
+ * exclusion set up-front lets the picker land on a candidate that is
+ * usable throughout the subtree.
+ *
+ * If the relation has no descendants, the behavior is identical to
+ * ChooseConstraintName.
+ *
+ * Returns a palloc'd string.
+ */
+char *
+ChooseConstraintNameForRelation(Oid relid, const char *name2,
+ const char *label, List *others)
+{
+ char *relname = get_rel_name(relid);
+ Oid relnamespace = get_rel_namespace(relid);
+ List *inheritors;
+ List *exclude;
+ ListCell *lc;
+ Relation conrel;
+
+ inheritors = find_all_inheritors(relid, NoLock, NULL);
+
+ /* Common case: no descendants -> identical to ChooseConstraintName. */
+ if (list_length(inheritors) <= 1)
+ {
+ list_free(inheritors);
+ return ChooseConstraintName(relname, name2, label, relnamespace, others);
+ }
+
+ exclude = list_copy(others);
+ conrel = table_open(ConstraintRelationId, AccessShareLock);
+
+ foreach(lc, inheritors)
+ {
+ Oid descoid = lfirst_oid(lc);
+ ScanKeyData skey;
+ SysScanDesc sscan;
+ HeapTuple tup;
+
+ /* The picker already scans the root's own namespace. */
+ if (descoid == relid)
+ continue;
+
+ ScanKeyInit(&skey,
+ Anum_pg_constraint_conrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(descoid));
+ sscan = systable_beginscan(conrel,
+ ConstraintRelidTypidNameIndexId,
+ true, NULL, 1, &skey);
+ while (HeapTupleIsValid(tup = systable_getnext(sscan)))
+ {
+ Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(tup);
+
+ exclude = lappend(exclude, pstrdup(NameStr(conform->conname)));
+ }
+ systable_endscan(sscan);
+ }
+
+ table_close(conrel, AccessShareLock);
+ list_free(inheritors);
+
+ return ChooseConstraintName(relname, name2, label, relnamespace, exclude);
+}
+
/*
* 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 a1845240a98..ed0bdc017e3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8149,10 +8149,9 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
if (!recursing)
{
Assert(conName == NULL);
- conName = ChooseConstraintName(RelationGetRelationName(rel),
- colName, "not_null",
- RelationGetNamespace(rel),
- NIL);
+ conName = ChooseConstraintNameForRelation(RelationGetRelid(rel),
+ colName, "not_null",
+ NIL);
}

constraint = makeNotNullConstraint(makeString(colName));
diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 1b7fedf1750..bce5dd017dd 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -262,6 +262,8 @@ 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 char *ChooseConstraintNameForRelation(Oid relid, const char *name2,
+ const char *label, List *others);

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 e54fec7fb57..64f98d80877 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -1708,6 +1708,101 @@ ERROR: cannot create primary key on column "a"
DETAIL: The constraint "nn" on column "a" of table "pp_nn_1", marked NO INHERIT, is incompatible with a primary key.
HINT: You might need to make the existing constraint inheritable using ALTER TABLE ... ALTER CONSTRAINT ... INHERIT.
DROP TABLE pp_nn;
+-- Constraint name picking on a partitioned root must consider names
+-- already in use on descendants in any schema, otherwise the auto-picked
+-- name collides with the leaf's existing constraint during recursion.
+CREATE SCHEMA other_schema;
+-- SET NOT NULL: auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ALTER COLUMN a SET NOT NULL;
+EXECUTE get_nnconstraint_info('{pp_nn, other_schema.pp_nn_1}');
+ tabname | conname | convalidated | conislocal | coninhcount
+----------------------+-------------------+--------------+------------+-------------
+ other_schema.pp_nn_1 | pp_nn_a_not_null | t | t | 0
+ other_schema.pp_nn_1 | pp_nn_a_not_null1 | t | f | 1
+ pp_nn | pp_nn_a_not_null1 | t | t | 0
+(3 rows)
+
+DROP TABLE pp_nn;
+-- ADD NOT NULL (constraint syntax): auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ADD NOT NULL a;
+EXECUTE get_nnconstraint_info('{pp_nn, other_schema.pp_nn_1}');
+ tabname | conname | convalidated | conislocal | coninhcount
+----------------------+-------------------+--------------+------------+-------------
+ other_schema.pp_nn_1 | pp_nn_a_not_null | t | t | 0
+ other_schema.pp_nn_1 | pp_nn_a_not_null1 | t | f | 1
+ pp_nn | pp_nn_a_not_null1 | t | t | 0
+(3 rows)
+
+DROP TABLE pp_nn;
+-- ADD CHECK: auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1 ADD CONSTRAINT pp_nn_a_check CHECK (a > -1);
+ALTER TABLE pp_nn ADD CHECK (a > 0);
+SELECT conrelid::regclass, conname, contype FROM pg_constraint
+WHERE conrelid IN ('pp_nn'::regclass, 'other_schema.pp_nn_1'::regclass)
+ AND contype = 'c'
+ORDER BY 1, 2;
+ conrelid | conname | contype
+----------------------+----------------+---------
+ pp_nn | pp_nn_a_check1 | c
+ other_schema.pp_nn_1 | pp_nn_a_check | c
+ other_schema.pp_nn_1 | pp_nn_a_check1 | c
+(3 rows)
+
+DROP TABLE pp_nn;
+-- Both ALTERs auto-named: a partition that shares its root's relname,
+-- even though it lives in a different schema, makes each auto-name
+-- pick the same name
+CREATE TABLE pp_nn_dup (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_dup
+ PARTITION OF pp_nn_dup FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_dup ADD CHECK (a > 100);
+ALTER TABLE pp_nn_dup ADD CHECK (a > 0);
+SELECT conrelid::regclass, conname FROM pg_constraint
+WHERE conrelid IN ('pp_nn_dup'::regclass, 'other_schema.pp_nn_dup'::regclass)
+ AND contype = 'c'
+ORDER BY 1, 2;
+ conrelid | conname
+------------------------+--------------------
+ pp_nn_dup | pp_nn_dup_a_check1
+ other_schema.pp_nn_dup | pp_nn_dup_a_check
+ other_schema.pp_nn_dup | pp_nn_dup_a_check1
+(3 rows)
+
+DROP TABLE pp_nn_dup;
+-- Multi-level partition tree: the collision is on a grandchild in a
+-- different schema (mid-level partition lives in the root's schema)
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE pp_nn_mid PARTITION OF pp_nn
+ FOR VALUES FROM (1) TO (10) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_leaf
+ PARTITION OF pp_nn_mid FOR VALUES FROM (1) TO (5);
+ALTER TABLE other_schema.pp_nn_leaf
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ALTER COLUMN a SET NOT NULL;
+EXECUTE get_nnconstraint_info('{pp_nn, pp_nn_mid, other_schema.pp_nn_leaf}');
+ tabname | conname | convalidated | conislocal | coninhcount
+-------------------------+-------------------+--------------+------------+-------------
+ other_schema.pp_nn_leaf | pp_nn_a_not_null | t | t | 0
+ other_schema.pp_nn_leaf | pp_nn_a_not_null1 | t | f | 1
+ pp_nn | pp_nn_a_not_null1 | t | t | 0
+ pp_nn_mid | pp_nn_a_not_null1 | t | f | 1
+(4 rows)
+
+DROP TABLE pp_nn;
+DROP SCHEMA other_schema;
-- Create table with NOT NULL INVALID constraint, for pg_upgrade.
CREATE TABLE notnull_tbl1_upg (a int, b int);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 1), (NULL, 2), (300, 3);
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index dc133b124bb..dc4150faa1e 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -1020,6 +1020,71 @@ ALTER TABLE pp_nn_1 ADD CONSTRAINT nn NOT NULL a NO INHERIT;
ALTER TABLE ONLY pp_nn ADD PRIMARY KEY (a);
DROP TABLE pp_nn;

+-- Constraint name picking on a partitioned root must consider names
+-- already in use on descendants in any schema, otherwise the auto-picked
+-- name collides with the leaf's existing constraint during recursion.
+CREATE SCHEMA other_schema;
+
+-- SET NOT NULL: auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ALTER COLUMN a SET NOT NULL;
+EXECUTE get_nnconstraint_info('{pp_nn, other_schema.pp_nn_1}');
+DROP TABLE pp_nn;
+
+-- ADD NOT NULL (constraint syntax): auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ADD NOT NULL a;
+EXECUTE get_nnconstraint_info('{pp_nn, other_schema.pp_nn_1}');
+DROP TABLE pp_nn;
+
+-- ADD CHECK: auto-named
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_1
+ PARTITION OF pp_nn FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_1 ADD CONSTRAINT pp_nn_a_check CHECK (a > -1);
+ALTER TABLE pp_nn ADD CHECK (a > 0);
+SELECT conrelid::regclass, conname, contype FROM pg_constraint
+WHERE conrelid IN ('pp_nn'::regclass, 'other_schema.pp_nn_1'::regclass)
+ AND contype = 'c'
+ORDER BY 1, 2;
+DROP TABLE pp_nn;
+
+-- Both ALTERs auto-named: a partition that shares its root's relname,
+-- even though it lives in a different schema, makes each auto-name
+-- pick the same name
+CREATE TABLE pp_nn_dup (a int) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_dup
+ PARTITION OF pp_nn_dup FOR VALUES FROM (1) TO (10);
+ALTER TABLE other_schema.pp_nn_dup ADD CHECK (a > 100);
+ALTER TABLE pp_nn_dup ADD CHECK (a > 0);
+SELECT conrelid::regclass, conname FROM pg_constraint
+WHERE conrelid IN ('pp_nn_dup'::regclass, 'other_schema.pp_nn_dup'::regclass)
+ AND contype = 'c'
+ORDER BY 1, 2;
+DROP TABLE pp_nn_dup;
+
+-- Multi-level partition tree: the collision is on a grandchild in a
+-- different schema (mid-level partition lives in the root's schema)
+CREATE TABLE pp_nn (a int) PARTITION BY RANGE (a);
+CREATE TABLE pp_nn_mid PARTITION OF pp_nn
+ FOR VALUES FROM (1) TO (10) PARTITION BY RANGE (a);
+CREATE TABLE other_schema.pp_nn_leaf
+ PARTITION OF pp_nn_mid FOR VALUES FROM (1) TO (5);
+ALTER TABLE other_schema.pp_nn_leaf
+ ADD CONSTRAINT pp_nn_a_not_null CHECK (a IS NOT NULL);
+ALTER TABLE pp_nn ALTER COLUMN a SET NOT NULL;
+EXECUTE get_nnconstraint_info('{pp_nn, pp_nn_mid, other_schema.pp_nn_leaf}');
+DROP TABLE pp_nn;
+DROP SCHEMA other_schema;
+
-- Create table with NOT NULL INVALID constraint, for pg_upgrade.
CREATE TABLE notnull_tbl1_upg (a int, b int);
INSERT INTO notnull_tbl1_upg VALUES (NULL, 1), (NULL, 2), (300, 3);

base-commit: 90354030b8fceecef9a1908fdefcccf0e02823fb
--
2.39.5 (Apple Git-154)