v6-0001-ensure-pg_dump-table-constraint-info-remain-the-s.patch

From 5d6da8791aa46d63e872c5a9f463b05bb2d0f9e8 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 2 Apr 2025 21:47:22 +0800
Subject: [PATCH v6 1/1] ensure pg_dump table constraint info remain the same

resolve case where parent constraint convalidated is false
and children constraint convalidated is true
all the dumped information is correct.

for example:

CREATE TABLE ttchk (a INTEGER);
ALTER TABLE ttchk ADD CONSTRAINT cc check (a is NOT NULL) NOT VALID;
CREATE TABLE ttchk_child(a INTEGER) INHERITS(ttchk);

we need dump as:

CREATE TABLE ttchk (a INTEGER);
CREATE TABLE public.ttchk_child (a integer, CONSTRAINT cc CHECK ((a IS NOT NULL))) INHERITS (public.ttchk);
ALTER TABLE public.ttchk ADD CONSTRAINT cc CHECK ((a IS NOT NULL)) NOT VALID;
---
 src/backend/catalog/heap.c            | 21 +++++++--
 src/backend/catalog/pg_constraint.c   |  9 ++++
 src/bin/pg_dump/pg_dump.c             | 66 ++++++++++++++++++++++++++-
 src/bin/pg_dump/pg_dump.h             |  1 +
 src/test/regress/expected/inherit.out |  2 +-
 5 files changed, 92 insertions(+), 7 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 734e34110fa..0bdcd59fa39 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2839,11 +2839,24 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
 		{
 			if (is_local)
 				con->conislocal = true;
-			else if (pg_add_s16_overflow(con->coninhcount, 1,
+			else
+			{
+				if(pg_add_s16_overflow(con->coninhcount, 1,
 										 &con->coninhcount))
-				ereport(ERROR,
-						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-						errmsg("too many inheritance parents"));
+					ereport(ERROR,
+							errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+							errmsg("too many inheritance parents"));
+
+				/*
+				 * If the children already has a valid constraint and we are
+				 * creating an invalid one on it.  The children's constraint
+				 * will remain valid as is, but it can longer be marked as
+				 * local.
+				*/
+				if (!is_initially_valid && con->convalidated &&
+					is_enforced && con->conenforced)
+					con->conislocal = false;
+			}
 		}
 
 		if (is_no_inherit)
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index c7ccc5bef32..48ac5a22e66 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -775,6 +775,15 @@ AdjustNotNullInheritance(Relation rel, AttrNumber attnum,
 				ereport(ERROR,
 						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 						errmsg("too many inheritance parents"));
+
+			/*
+			 * If the children already has a valid constraint and we are
+			 * creating an invalid one on it.  The children's constraint will
+			 * remain valid as is, but it can longer be marked as local.
+			*/
+			if (is_notvalid && conform->convalidated && conform->conenforced)
+				conform->conislocal = false;
+
 			changed = true;
 		}
 		else if (!conform->conislocal)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 96e45f888d4..282e774f3cb 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -401,6 +401,7 @@ static void setupDumpWorker(Archive *AH);
 static TableInfo *getRootTableInfo(const TableInfo *tbinfo);
 static bool forcePartitionRootLoad(const TableInfo *tbinfo);
 static void read_dump_filters(const char *filename, DumpOptions *dopt);
+static bool constr_dump_force_not_seperate(Archive *fout, Oid conrelid, char conkind);
 
 
 int
@@ -7950,6 +7951,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 				constrinfo->conperiod = *(PQgetvalue(res, j, i_conperiod)) == 't';
 				constrinfo->conislocal = true;
 				constrinfo->separate = true;
+				constrinfo->force_notseperate = false;
 
 				indxinfo[j].indexconstraint = constrinfo->dobj.dumpId;
 				if (relstats != NULL)
@@ -8167,6 +8169,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
 		constrinfo[j].condeferred = false;
 		constrinfo[j].conislocal = true;
 		constrinfo[j].separate = true;
+		constrinfo[j].force_notseperate = false;
 
 		/*
 		 * Restoring an FK that points to a partitioned table requires that
@@ -8306,6 +8309,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 		constrinfo[i].conislocal = true;
 
 		constrinfo[i].separate = !validated;
+		constrinfo[i].force_notseperate = false;
 
 		/*
 		 * Make the domain depend on the constraint, ensuring it won't be
@@ -9559,6 +9563,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		int			i_consrc;
 		int			i_conislocal;
 		int			i_convalidated;
+		int			i_coninhcount;
 
 		pg_log_info("finding table check constraints");
 
@@ -9566,7 +9571,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		appendPQExpBuffer(q,
 						  "SELECT c.tableoid, c.oid, conrelid, conname, "
 						  "pg_catalog.pg_get_constraintdef(c.oid) AS consrc, "
-						  "conislocal, convalidated "
+						  "conislocal, convalidated, coninhcount "
 						  "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n"
 						  "JOIN pg_catalog.pg_constraint c ON (src.tbloid = c.conrelid)\n"
 						  "WHERE contype = 'c' "
@@ -9585,6 +9590,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		i_consrc = PQfnumber(res, "consrc");
 		i_conislocal = PQfnumber(res, "conislocal");
 		i_convalidated = PQfnumber(res, "convalidated");
+		i_coninhcount = PQfnumber(res, "coninhcount");
 
 		/* As above, this loop iterates once per table, not once per row */
 		curtblindx = -1;
@@ -9653,6 +9659,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 
 				constrs[j].dobj.dump = tbinfo->dobj.dump;
 
+				constrs[j].force_notseperate = false;
+				if(validated && (atoi(PQgetvalue(res, j, i_coninhcount)) > 0))
+					constrs[j].force_notseperate = constr_dump_force_not_seperate(fout, conrelid, 'c');
+
 				/*
 				 * Mark the constraint as needing to appear before the table
 				 * --- this is so that any other dependencies of the
@@ -9681,6 +9691,54 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	destroyPQExpBuffer(checkoids);
 }
 
+/*
+ * (XXX need refinement)
+ * comments for cases: parent constraint is not valid, child is valid:
+ * child's constraint conislocal is false, pg_dump won't dump it, later creating
+ * parent constraint will also create children's.  pg_dump will dump invalidated
+ * constraint seperately.
+ * So: for cases: parent constraint is not valid, child is valid, pu_dump's
+ * output will make parent and childconstraint is not valid.
+ *
+ * To resolve this issue:
+ * The child's (conislocal is false) constraint should be dumped within CREATE
+ * TABLE statement.  This ensures ALTER TABLE ADD CONSTRAINT on the parent
+ * cascades to the child, no issues occur.
+ *
+ * the caller should make sure 'conrelid' have inherited constraint!
+ *
+*/
+static bool
+constr_dump_force_not_seperate(Archive *fout, Oid conrelid, char conkind)
+{
+	int64		cnt = 0;
+	PQExpBuffer qs = createPQExpBuffer();
+	PGresult   *res;
+
+	appendPQExpBuffer(qs,
+						"SELECT count(*) as cnt\n"
+						"FROM pg_constraint pc\n"
+						"JOIN pg_inherits pi ON pi.inhrelid = pc.conrelid\n"
+						"JOIN pg_constraint pc1 ON pc1.conrelid = pi.inhparent\n"
+						"WHERE pc.contype = '%c' "
+						"AND pc.convalidated AND NOT pc1.convalidated "
+						"AND pc.conrelid = %u ",
+						conkind, conrelid);
+
+	res = ExecuteSqlQuery(fout, qs->data, PGRES_TUPLES_OK);
+
+	Assert(PQntuples(res) == 1);
+
+	cnt = strtoi64(PQgetvalue(res, 0, PQfnumber(res, "cnt")), NULL, 10);
+
+	destroyPQExpBuffer(qs);
+
+	if (cnt > 0)
+		return true;
+	else
+		return false;
+}
+
 /*
  * Based on the getTableAttrs query's row corresponding to one column, set
  * the name and flags to handle a not-null constraint for that column in
@@ -16843,6 +16901,10 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 									  dopt->binary_upgrade ||
 									  tbinfo->ispartition));
 
+					if (!print_notnull && tbinfo->numParents > 0 &&
+						tbinfo->notnull_constrs[j] != NULL)
+						print_notnull = constr_dump_force_not_seperate(fout, tbinfo->dobj.catId.oid, 'n');
+
 					if (print_notnull)
 					{
 						if (tbinfo->notnull_constrs[j][0] == '\0')
@@ -16913,7 +16975,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 				ConstraintInfo *constr = &(tbinfo->checkexprs[j]);
 
 				if (constr->separate ||
-					(!constr->conislocal && !tbinfo->ispartition))
+					(!constr->conislocal && !tbinfo->ispartition && !constr->force_notseperate))
 					continue;
 
 				if (actual_atts == 0)
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 195b4495768..1720c621abe 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -518,6 +518,7 @@ typedef struct _constraintInfo
 	bool		conperiod;		/* true if the constraint is WITHOUT OVERLAPS */
 	bool		conislocal;		/* true if constraint has local definition */
 	bool		separate;		/* true if must dump as separate item */
+	bool		force_notseperate;	/* true if must not dump as separate item */
 } ConstraintInfo;
 
 typedef struct _procLangInfo
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index c01e9d5244f..2cb2c482ea5 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1558,7 +1558,7 @@ order by 1, 2;
          relname         |       conname        | convalidated | conislocal | coninhcount | connoinherit 
 -------------------------+----------------------+--------------+------------+-------------+--------------
  invalid_check_con       | inh_check_constraint | f            | t          |           0 | f
- invalid_check_con_child | inh_check_constraint | t            | t          |           1 | f
+ invalid_check_con_child | inh_check_constraint | t            | f          |           1 | f
 (2 rows)
 
 -- We don't drop the invalid_check_con* tables, to test dump/reload with
-- 
2.34.1