From 7c7b79cec94cc8c23e90582667f49be4187849f4 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 30 Jul 2026 16:03:49 +0800
Subject: [PATCH v2] Add find_all_inheritors_ordered()

Add an inheritance traversal that guarantees every ancestor appears before
its descendants.  find_all_inheritors() does not provide this ordering when
multiple inheritance creates both direct and indirect paths to a relation.

Use Kahn's topological sorting algorithm to produce the ordered list without
recursion.

Use the new function to simplify the inherited CHECK constraint
enforceability handling committed by 0cd17fdd3c0.  Since parents are
processed before their children, checking the current state of a child's
direct parents is sufficient.

Extend the direct-plus-indirect inheritance test with another intermediate
level.

Author: Chao Li <lic@highgo.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Reviewed-by: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/57100518-3FF1-48AD-B550-47B9A3E88688@gmail.com
---
 src/backend/catalog/pg_inherits.c     | 138 ++++++++++++++++++++++++++
 src/backend/commands/tablecmds.c      | 110 ++++----------------
 src/include/catalog/pg_inherits.h     |   1 +
 src/test/regress/expected/inherit.out |   9 +-
 src/test/regress/sql/inherit.sql      |   3 +-
 5 files changed, 169 insertions(+), 92 deletions(-)

diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index 23fb7886cfb..c80077dc643 100644
--- a/src/backend/catalog/pg_inherits.c
+++ b/src/backend/catalog/pg_inherits.c
@@ -29,6 +29,7 @@
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/hsearch.h"
+#include "utils/memutils.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 
@@ -41,6 +42,16 @@ typedef struct SeenRelsEntry
 	int			list_index;		/* its position in output list(s) */
 } SeenRelsEntry;
 
+/*
+ * Entry of a hash table used in find_all_inheritors_ordered.
+ */
+typedef struct OrderedSeenRelsEntry
+{
+	Oid			rel_id;
+	List	   *children;
+	int			indegree;
+}			OrderedSeenRelsEntry;
+
 /*
  * find_inheritance_children
  *
@@ -335,6 +346,133 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents)
 	return rels_list;
 }
 
+/*
+ * find_all_inheritors_ordered -
+ *		Same as find_all_inheritors(), except that an ancestor is always listed
+ *		before its descendants.
+ *
+ * The ordering is produced using Kahn's topological sorting algorithm.
+ * Unlike find_all_inheritors(), this function reports an error if the
+ * inheritance graph contains a cycle, since no such ordering is possible.
+ * Concurrent ALTER TABLE ... INHERIT commands can create cycles despite
+ * the circularity check in ATExecAddInherit().  Callers that need to tolerate
+ * cycles should use find_all_inheritors() instead.
+ */
+List *
+find_all_inheritors_ordered(Oid parentrelId, LOCKMODE lockmode)
+{
+	MemoryContext temp_context;
+	MemoryContext old_context;
+	HTAB	   *seen_rels;
+	HASHCTL		ctl;
+	List	   *agenda;
+	List	   *worklist = NIL;
+	List	   *ordered = NIL;
+	List	   *result = NIL;
+	ListCell   *lc;
+	OrderedSeenRelsEntry *node;
+	bool		found;
+
+	/* Use a temporary memory context to simplify cleanup */
+	temp_context = AllocSetContextCreate(CurrentMemoryContext,
+										 "find_all_inheritors_ordered",
+										 ALLOCSET_SMALL_SIZES);
+	old_context = MemoryContextSwitchTo(temp_context);
+
+	ctl.keysize = sizeof(Oid);
+	ctl.entrysize = sizeof(OrderedSeenRelsEntry);
+	ctl.hcxt = temp_context;
+
+	seen_rels = hash_create("find_all_inheritors_ordered temporary table",
+							32, /* start small and extend */
+							&ctl,
+							HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
+
+	node = hash_search(seen_rels, &parentrelId, HASH_ENTER, &found);
+	Assert(!found);
+	node->children = NIL;
+	node->indegree = 0;
+
+	/*
+	 * Use the agenda to find every descendant and record every inheritance
+	 * edge.  The indegree is the number of direct parents within this
+	 * inheritance graph.
+	 */
+	agenda = list_make1_oid(parentrelId);
+	foreach(lc, agenda)
+	{
+		Oid			current_oid = lfirst_oid(lc);
+		List	   *children;
+		OrderedSeenRelsEntry *current;
+
+		current = hash_search(seen_rels, &current_oid, HASH_FIND, NULL);
+		Assert(current != NULL);
+
+		children = find_inheritance_children(current_oid, lockmode);
+		foreach_oid(child_oid, children)
+		{
+			node = hash_search(seen_rels, &child_oid, HASH_ENTER, &found);
+			if (!found)
+			{
+				node->children = NIL;
+				node->indegree = 0;
+				agenda = lappend_oid(agenda, child_oid);
+			}
+
+			current->children = lappend_oid(current->children, child_oid);
+			node->indegree++;
+		}
+		list_free(children);
+	}
+
+	/*
+	 * Emit nodes in topological order.  A node enters the worklist only after
+	 * all of its direct parents have been emitted.
+	 */
+	foreach_oid(rel_oid, agenda)
+	{
+		node = hash_search(seen_rels, &rel_oid, HASH_FIND, NULL);
+		Assert(node != NULL);
+		if (node->indegree == 0)
+			worklist = lappend_oid(worklist, rel_oid);
+	}
+
+	/*
+	 * Move nodes from the worklist to the output list, and add their children
+	 * to the worklist when all of their parents have been emitted.
+	 */
+	foreach_oid(rel_oid, worklist)
+	{
+		node = hash_search(seen_rels, &rel_oid, HASH_FIND, NULL);
+		Assert(node != NULL);
+		ordered = lappend_oid(ordered, rel_oid);
+
+		foreach_oid(child_oid, node->children)
+		{
+			OrderedSeenRelsEntry *child;
+
+			child = hash_search(seen_rels, &child_oid, HASH_FIND, NULL);
+			Assert(child != NULL);
+			Assert(child->indegree > 0);
+			if (--child->indegree == 0)
+				worklist = lappend_oid(worklist, child_oid);
+		}
+	}
+
+	/* Any nodes not emitted are in a cycle or depend on one. */
+	if (list_length(ordered) != list_length(agenda))
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+				errmsg("cannot order relations with circular inheritance"),
+				errhint("Use ALTER TABLE ... NO INHERIT to break the cycle."));
+
+	MemoryContextSwitchTo(old_context);
+	result = list_copy(ordered);
+	MemoryContextDelete(temp_context);
+
+	return result;
+}
+
 
 /*
  * has_subclass - does this relation have any children?
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fd144d783d9..6024f804bfc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -415,7 +415,6 @@ static bool ATExecAlterFKConstrEnforceability(List **wqueue, ATAlterConstraint *
 static bool ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 												 Relation conrel, HeapTuple contuple,
 												 bool recurse, bool recursing,
-												 List *changing_conids,
 												 LOCKMODE lockmode);
 static bool ATExecAlterConstrDeferrability(List **wqueue, ATAlterConstraint *cmdcon,
 										   Relation conrel, Relation tgrel, Relation rel,
@@ -438,7 +437,6 @@ static void AlterFKConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint
 static void AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon,
 												  Relation conrel, Oid conrelid,
 												  bool recurse, bool recursing,
-												  List *changing_conids,
 												  LOCKMODE lockmode);
 static void AlterConstrDeferrabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon,
 											Relation conrel, Relation tgrel, Relation rel,
@@ -448,7 +446,6 @@ static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation
 											 HeapTuple contuple);
 static bool ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel,
 												HeapTuple contuple,
-												List *changing_conids,
 												Oid *enforced_parentoid);
 static ObjectAddress ATExecValidateConstraint(List **wqueue,
 											  Relation rel, char *constrName,
@@ -12478,7 +12475,7 @@ ATExecAlterConstraintInternal(List **wqueue, ATAlterConstraint *cmdcon,
 		else if (currcon->contype == CONSTRAINT_CHECK)
 			changed = ATExecAlterCheckConstrEnforceability(wqueue, cmdcon, conrel,
 														   contuple, recurse, false,
-														   NIL, lockmode);
+														   lockmode);
 	}
 	else if (cmdcon->alterDeferrability &&
 			 ATExecAlterConstrDeferrability(wqueue, cmdcon, conrel, tgrel, rel,
@@ -12666,7 +12663,6 @@ static bool
 ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 									 Relation conrel, HeapTuple contuple,
 									 bool recurse, bool recursing,
-									 List *changing_conids,
 									 LOCKMODE lockmode)
 {
 	Form_pg_constraint currcon;
@@ -12708,7 +12704,6 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 	if (!cmdcon->is_enforced &&
 		(!recursing || !rel->rd_rel->relispartition) &&
 		ATCheckCheckConstrHasEnforcedParent(conrel, rel, contuple,
-											changing_conids,
 											&enforced_parentoid))
 	{
 		if (!recursing)
@@ -12735,6 +12730,14 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 		updatecon.is_enforced = target_enforced;
 		AlterConstrUpdateConstraintEntry(&updatecon, conrel, contuple);
 		changed = true;
+
+		/*
+		 * A later descendant must see this change when checking its direct
+		 * parents.  The ordered inheritance traversal guarantees that all
+		 * affected parents are processed before their children.
+		 */
+		if (!cmdcon->is_enforced)
+			CommandCounterIncrement();
 	}
 
 	/*
@@ -12755,55 +12758,20 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 		 */
 		if (!recursing && !currcon->connoinherit)
 		{
-			Assert(changing_conids == NIL);
-
-			children = find_all_inheritors(RelationGetRelid(rel),
-										   lockmode, NULL);
+			children = find_all_inheritors_ordered(RelationGetRelid(rel),
+												   lockmode);
 
 			/*
-			 * When setting NOT ENFORCED, build the set of equivalent CHECK
-			 * constraints that this command will attempt to change before
-			 * visiting descendants. The root itself has already been checked
-			 * above.
+			 * If we are told not to recurse, there had better not be any
+			 * child tables, because we can't change constraint enforceability
+			 * on the parent unless we have changed enforceability for all
+			 * children.
 			 */
-			if (!cmdcon->is_enforced)
-				changing_conids = list_make1_oid(currcon->oid);
-
-			foreach_oid(childoid, children)
-			{
-				if (childoid == RelationGetRelid(rel))
-					continue;
-
-				/*
-				 * If we are told not to recurse, there had better not be any
-				 * child tables, because we can't change constraint
-				 * enforceability on the parent unless we have changed
-				 * enforceability for all child.
-				 */
-				if (!recurse)
-					ereport(ERROR,
-							errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-							errmsg("constraint must be altered on child tables too"),
-							errhint("Do not specify the ONLY keyword."));
-
-				/*
-				 * It is sufficient to look up the constraint by name here.
-				 * Supported DDL ensures that inheritable CHECK constraints
-				 * with the same name have equivalent definitions when they
-				 * are propagated to children or when inheritance is
-				 * established.  All descendants returned by
-				 * find_all_inheritors must have this constraint: inherited
-				 * CHECK constraints propagate to all children at
-				 * inheritance-link creation time and cannot be dropped
-				 * independently on child tables.
-				 */
-				if (!cmdcon->is_enforced)
-					changing_conids =
-						list_append_unique_oid(changing_conids,
-											   get_relation_constraint_oid(childoid,
-																		   cmdcon->conname,
-																		   false));
-			}
+			if (!recurse && list_length(children) > 1)
+				ereport(ERROR,
+						errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+						errmsg("constraint must be altered on child tables too"),
+						errhint("Do not specify the ONLY keyword."));
 		}
 
 		foreach_oid(childoid, children)
@@ -12813,7 +12781,6 @@ ATExecAlterCheckConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
 
 			AlterCheckConstrEnforceabilityRecurse(wqueue, cmdcon, conrel,
 												  childoid, false, true,
-												  changing_conids,
 												  lockmode);
 		}
 	}
@@ -12865,7 +12832,6 @@ static void
 AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon,
 									  Relation conrel, Oid conrelid,
 									  bool recurse, bool recursing,
-									  List *changing_conids,
 									  LOCKMODE lockmode)
 {
 	SysScanDesc pscan;
@@ -12895,21 +12861,18 @@ AlterCheckConstrEnforceabilityRecurse(List **wqueue, ATAlterConstraint *cmdcon,
 					   cmdcon->conname, get_rel_name(conrelid)));
 
 	ATExecAlterCheckConstrEnforceability(wqueue, cmdcon, conrel, childtup,
-										 recurse, recursing, changing_conids,
-										 lockmode);
+										 recurse, recursing, lockmode);
 
 	systable_endscan(pscan);
 }
 
 /*
  * When setting an inherited CHECK constraint to NOT ENFORCED, look for a
- * matching parent constraint that remains ENFORCED and is not part of the same
- * ALTER.
+ * matching parent constraint that remains ENFORCED.
  */
 static bool
 ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel,
 									HeapTuple contuple,
-									List *changing_conids,
 									Oid *enforced_parentoid)
 {
 	Form_pg_constraint currcon;
@@ -12939,7 +12902,6 @@ ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel,
 	while (HeapTupleIsValid(inheritsTuple = systable_getnext(scan)))
 	{
 		Oid			parentoid;
-		Relation	parentrel = NULL;
 		SysScanDesc pscan;
 		ScanKeyData pkey[3];
 		HeapTuple	parenttup;
@@ -12983,41 +12945,13 @@ ATCheckCheckConstrHasEnforcedParent(Relation conrel, Relation rel,
 					 RelationGetRelationName(rel),
 					 NameStr(parentcon->conname));
 
-			/*
-			 * A parent listed in changing_conids is being changed by the same
-			 * ALTER, but it may not have been updated yet.  For regular
-			 * inheritance, recurse upward to check whether an equivalent
-			 * enforced parent outside the ALTER will make it remain enforced.
-			 * Partitions cannot have multiple parents, so they do not need
-			 * this check.
-			 */
-			if (!rel->rd_rel->relispartition &&
-				list_member_oid(changing_conids, parentcon->oid))
-			{
-				Oid			parent_enforced_parentoid = InvalidOid;
-
-				if (parentrel == NULL)
-					parentrel = table_open(parentoid, NoLock);
-
-				if (!ATCheckCheckConstrHasEnforcedParent(conrel,
-														 parentrel,
-														 parenttup,
-														 changing_conids,
-														 &parent_enforced_parentoid))
-					continue;
-			}
-
 			*enforced_parentoid = parentoid;
-			if (parentrel != NULL)
-				table_close(parentrel, NoLock);
 			systable_endscan(pscan);
 			systable_endscan(scan);
 			table_close(inhrel, AccessShareLock);
 			return true;
 		}
 
-		if (parentrel != NULL)
-			table_close(parentrel, NoLock);
 		systable_endscan(pscan);
 	}
 
diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.h
index cc874abaabb..66a811e50fa 100644
--- a/src/include/catalog/pg_inherits.h
+++ b/src/include/catalog/pg_inherits.h
@@ -58,6 +58,7 @@ extern List *find_inheritance_children_extended(Oid parentrelId, bool omit_detac
 
 extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode,
 								 List **numparents);
+extern List *find_all_inheritors_ordered(Oid parentrelId, LOCKMODE lockmode);
 extern bool has_subclass(Oid relationId);
 extern bool has_superclass(Oid relationId);
 extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId);
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 0136aa53c96..38b057a5e8e 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1554,7 +1554,8 @@ drop cascades to table p1_c1
 create table gp(a int constraint gp_a_check check (a > 0) enforced);
 create table p1_c1() inherits (gp);
 create table p1() inherits (gp);
-alter table p1_c1 inherit p1;
+create table p2() inherits (p1);
+alter table p1_c1 inherit p2;
 alter table gp alter constraint gp_a_check not enforced; --ok
 select  conname, conenforced, convalidated, conrelid::regclass
 from    pg_constraint
@@ -1565,11 +1566,13 @@ order by conrelid::regclass::text collate "C";
  gp_a_check | f           | f            | gp
  gp_a_check | f           | f            | p1
  gp_a_check | f           | f            | p1_c1
-(3 rows)
+ gp_a_check | f           | f            | p2
+(4 rows)
 
 drop table gp cascade;
-NOTICE:  drop cascades to 2 other objects
+NOTICE:  drop cascades to 3 other objects
 DETAIL:  drop cascades to table p1
+drop cascades to table p2
 drop cascades to table p1_c1
 --for "no inherit" check constraint, it will not recurse to child table
 create table p1(f1 int constraint p1_a_check check (f1 > 0) no inherit not enforced);
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 072fca13c13..db4f4abc709 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -579,7 +579,8 @@ drop table gp cascade;
 create table gp(a int constraint gp_a_check check (a > 0) enforced);
 create table p1_c1() inherits (gp);
 create table p1() inherits (gp);
-alter table p1_c1 inherit p1;
+create table p2() inherits (p1);
+alter table p1_c1 inherit p2;
 alter table gp alter constraint gp_a_check not enforced; --ok
 select  conname, conenforced, convalidated, conrelid::regclass
 from    pg_constraint
-- 
2.50.1 (Apple Git-155)

