v24-0003-Move-EquivalenceClass-indexes-to-PlannerInfo.patch

application/octet-stream

Filename: v24-0003-Move-EquivalenceClass-indexes-to-PlannerInfo.patch
Type: application/octet-stream
Part: 2
Message: Re: [PoC] Reducing planning time when tables have many partitions

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v24-0003
Subject: Move EquivalenceClass indexes to PlannerInfo
File+
src/backend/nodes/outfuncs.c 0 2
src/backend/nodes/readfuncs.c 0 2
src/backend/optimizer/path/equivclass.c 41 42
src/backend/optimizer/plan/analyzejoins.c 8 8
src/backend/optimizer/util/inherit.c 0 7
src/backend/optimizer/util/relnode.c 6 0
src/include/nodes/parsenodes.h 0 6
src/include/nodes/pathnodes.h 24 0
src/tools/pgindent/typedefs.list 1 0
From 79d246c59b1b6bc077c44805c7e1d7d1c190828a Mon Sep 17 00:00:00 2001
From: Yuya Watari <watari.yuya@gmail.com>
Date: Fri, 19 Jan 2024 11:43:29 +0900
Subject: [PATCH v24 3/3] Move EquivalenceClass indexes to PlannerInfo

In the previous commit, the indexes, namely ec_[source|derive]_indexes,
were in RestrictInfo. This was a workaround because RelOptInfo was the
best place but some RelOptInfos can be NULL and we cannot store indexes
for them.

This commit introduces a new struct, EquivalenceClassIndexes. This
struct exists in PlannerInfo and holds our indexes. This change prevents
a serialization problem with RestirctInfo in the previous commit.
---
 src/backend/nodes/outfuncs.c              |  2 -
 src/backend/nodes/readfuncs.c             |  2 -
 src/backend/optimizer/path/equivclass.c   | 83 +++++++++++------------
 src/backend/optimizer/plan/analyzejoins.c | 16 ++---
 src/backend/optimizer/util/inherit.c      |  7 --
 src/backend/optimizer/util/relnode.c      |  6 ++
 src/include/nodes/parsenodes.h            |  6 --
 src/include/nodes/pathnodes.h             | 24 +++++++
 src/tools/pgindent/typedefs.list          |  1 +
 9 files changed, 80 insertions(+), 67 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c61110c200..b08a4789c4 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -567,8 +567,6 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 	WRITE_BOOL_FIELD(inh);
 	WRITE_BOOL_FIELD(inFromCl);
 	WRITE_NODE_FIELD(securityQuals);
-	WRITE_BITMAPSET_FIELD(eclass_source_indexes);
-	WRITE_BITMAPSET_FIELD(eclass_derive_indexes);
 }
 
 static void
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index e3518c602b..b1e2f2b440 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -431,8 +431,6 @@ _readRangeTblEntry(void)
 	READ_BOOL_FIELD(inh);
 	READ_BOOL_FIELD(inFromCl);
 	READ_NODE_FIELD(securityQuals);
-	READ_BITMAPSET_FIELD(eclass_source_indexes);
-	READ_BITMAPSET_FIELD(eclass_derive_indexes);
 
 	READ_DONE();
 }
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 17fffc4087..4fb1ecff03 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -607,10 +607,10 @@ add_eq_source(PlannerInfo *root, EquivalenceClass *ec, RestrictInfo *rinfo)
 	i = -1;
 	while ((i = bms_next_member(rinfo->clause_relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
-		rte->eclass_source_indexes = bms_add_member(rte->eclass_source_indexes,
-													source_idx);
+		index->source_indexes = bms_add_member(index->source_indexes,
+											   source_idx);
 	}
 }
 
@@ -631,10 +631,10 @@ add_eq_derive(PlannerInfo *root, EquivalenceClass *ec, RestrictInfo *rinfo)
 	i = -1;
 	while ((i = bms_next_member(rinfo->clause_relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
-		rte->eclass_derive_indexes = bms_add_member(rte->eclass_derive_indexes,
-													derive_idx);
+		index->derive_indexes = bms_add_member(index->derive_indexes,
+											   derive_idx);
 	}
 }
 
@@ -673,22 +673,22 @@ update_clause_relids(PlannerInfo *root, RestrictInfo *rinfo,
 	i = -1;
 	while ((i = bms_next_member(removing_relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
 		if (rinfo->eq_sources_index != -1)
 		{
 			Assert(bms_is_member(rinfo->eq_sources_index,
-								 rte->eclass_source_indexes));
-			rte->eclass_source_indexes =
-				bms_del_member(rte->eclass_source_indexes,
+								 index->source_indexes));
+			index->source_indexes =
+				bms_del_member(index->source_indexes,
 							   rinfo->eq_sources_index);
 		}
 		if (rinfo->eq_derives_index != -1)
 		{
 			Assert(bms_is_member(rinfo->eq_derives_index,
-								 rte->eclass_derive_indexes));
-			rte->eclass_derive_indexes =
-				bms_del_member(rte->eclass_derive_indexes,
+								 index->derive_indexes));
+			index->derive_indexes =
+				bms_del_member(index->derive_indexes,
 							   rinfo->eq_derives_index);
 		}
 	}
@@ -702,22 +702,22 @@ update_clause_relids(PlannerInfo *root, RestrictInfo *rinfo,
 	i = -1;
 	while ((i = bms_next_member(adding_relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
 		if (rinfo->eq_sources_index != -1)
 		{
 			Assert(!bms_is_member(rinfo->eq_sources_index,
-								  rte->eclass_source_indexes));
-			rte->eclass_source_indexes =
-				bms_add_member(rte->eclass_source_indexes,
+								  index->source_indexes));
+			index->source_indexes =
+				bms_add_member(index->source_indexes,
 							   rinfo->eq_sources_index);
 		}
 		if (rinfo->eq_derives_index != -1)
 		{
 			Assert(!bms_is_member(rinfo->eq_derives_index,
-								  rte->eclass_derive_indexes));
-			rte->eclass_derive_indexes =
-				bms_add_member(rte->eclass_derive_indexes,
+								  index->derive_indexes));
+			index->derive_indexes =
+				bms_add_member(index->derive_indexes,
 							   rinfo->eq_derives_index);
 		}
 	}
@@ -731,17 +731,17 @@ update_clause_relids(PlannerInfo *root, RestrictInfo *rinfo,
 	i = -1;
 	while ((i = bms_next_member(common_relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
 		if (rinfo->eq_sources_index != -1)
 		{
 			Assert(bms_is_member(rinfo->eq_sources_index,
-								 rte->eclass_source_indexes));
+								 index->source_indexes));
 		}
 		if (rinfo->eq_derives_index != -1)
 		{
 			Assert(bms_is_member(rinfo->eq_derives_index,
-								 rte->eclass_derive_indexes));
+								 index->derive_indexes));
 		}
 	}
 	bms_free(common_relids);
@@ -3777,9 +3777,9 @@ get_ec_source_indexes(PlannerInfo *root, EquivalenceClass *ec, Relids relids)
 
 	while ((i = bms_next_member(relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
-		rel_esis = bms_add_members(rel_esis, rte->eclass_source_indexes);
+		rel_esis = bms_add_members(rel_esis, index->source_indexes);
 	}
 
 #ifdef USE_ASSERT_CHECKING
@@ -3815,7 +3815,7 @@ get_ec_source_indexes_strict(PlannerInfo *root, EquivalenceClass *ec,
 
 	if (i >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
 		/*
 		 * bms_intersect to the first relation to try to keep the resulting
@@ -3824,12 +3824,12 @@ get_ec_source_indexes_strict(PlannerInfo *root, EquivalenceClass *ec,
 		 * more words than the other.
 		 */
 		esis = bms_intersect(ec->ec_source_indexes,
-							 rte->eclass_source_indexes);
+							 index->source_indexes);
 
 		while ((i = bms_next_member(relids, i)) >= 0)
 		{
-			rte = root->simple_rte_array[i];
-			esis = bms_int_members(esis, rte->eclass_source_indexes);
+			index = &root->eclass_indexes_array[i];
+			esis = bms_int_members(esis, index->source_indexes);
 		}
 	}
 
@@ -3866,9 +3866,9 @@ get_ec_derive_indexes(PlannerInfo *root, EquivalenceClass *ec, Relids relids)
 
 	while ((i = bms_next_member(relids, i)) >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
-		rel_edis = bms_add_members(rel_edis, rte->eclass_derive_indexes);
+		rel_edis = bms_add_members(rel_edis, index->derive_indexes);
 	}
 
 #ifdef USE_ASSERT_CHECKING
@@ -3904,7 +3904,7 @@ get_ec_derive_indexes_strict(PlannerInfo *root, EquivalenceClass *ec,
 
 	if (i >= 0)
 	{
-		RangeTblEntry *rte = root->simple_rte_array[i];
+		EquivalenceClassIndexes *index = &root->eclass_indexes_array[i];
 
 		/*
 		 * bms_intersect to the first relation to try to keep the resulting
@@ -3913,12 +3913,12 @@ get_ec_derive_indexes_strict(PlannerInfo *root, EquivalenceClass *ec,
 		 * more words than the other.
 		 */
 		edis = bms_intersect(ec->ec_derive_indexes,
-							 rte->eclass_derive_indexes);
+							 index->derive_indexes);
 
 		while ((i = bms_next_member(relids, i)) >= 0)
 		{
-			rte = root->simple_rte_array[i];
-			edis = bms_int_members(edis, rte->eclass_derive_indexes);
+			index = &root->eclass_indexes_array[i];
+			edis = bms_int_members(edis, index->derive_indexes);
 		}
 	}
 
@@ -3941,9 +3941,8 @@ get_ec_derive_indexes_strict(PlannerInfo *root, EquivalenceClass *ec,
 /*
  * verify_eclass_indexes
  *		Verify that there are no missing references between RestrictInfos and
- *		EquivalenceMember's indexes, namely eclass_source_indexes and
- *		eclass_derive_indexes. If you modify these indexes, you should check
- *		them with this function.
+ *		EquivalenceMember's indexes, namely source_indexes and derive_indexes.
+ *		If you modify these indexes, you should check them with this function.
  */
 void
 verify_eclass_indexes(PlannerInfo *root, EquivalenceClass *ec)
@@ -3952,7 +3951,7 @@ verify_eclass_indexes(PlannerInfo *root, EquivalenceClass *ec)
 
 	/*
 	 * All RestrictInfos in root->eq_sources must have references to
-	 * eclass_source_indexes.
+	 * source_indexes.
 	 */
 	foreach(lc, root->eq_sources)
 	{
@@ -3969,13 +3968,13 @@ verify_eclass_indexes(PlannerInfo *root, EquivalenceClass *ec)
 		while ((k = bms_next_member(rinfo->clause_relids, k)) >= 0)
 		{
 			/* must have a reference */
-			Assert(bms_is_member(index, root->simple_rte_array[k]->eclass_source_indexes));
+			Assert(bms_is_member(index, root->eclass_indexes_array[k].source_indexes));
 		}
 	}
 
 	/*
 	 * All RestrictInfos in root->eq_derives must have references to
-	 * eclass_derive_indexes.
+	 * derive_indexes.
 	 */
 	foreach(lc, root->eq_derives)
 	{
@@ -3992,7 +3991,7 @@ verify_eclass_indexes(PlannerInfo *root, EquivalenceClass *ec)
 		while ((k = bms_next_member(rinfo->clause_relids, k)) >= 0)
 		{
 			/* must have a reference */
-			Assert(bms_is_member(index, root->simple_rte_array[k]->eclass_derive_indexes));
+			Assert(bms_is_member(index, root->eclass_indexes_array[k].derive_indexes));
 		}
 	}
 }
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 13e987493b..d2fdc2649a 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -1686,11 +1686,11 @@ update_eclasses(PlannerInfo *root, EquivalenceClass *ec, int from, int to)
 		j = -1;
 		while ((j = bms_next_member(rinfo->clause_relids, j)) >= 0)
 		{
-			RangeTblEntry *rte = root->simple_rte_array[j];
+			EquivalenceClassIndexes *index = &root->eclass_indexes_array[j];
 
-			Assert(bms_is_member(i, rte->eclass_derive_indexes));
-			rte->eclass_derive_indexes =
-				bms_del_member(rte->eclass_derive_indexes, i);
+			Assert(bms_is_member(i, index->derive_indexes));
+			index->derive_indexes =
+				bms_del_member(index->derive_indexes, i);
 		}
 
 		/*
@@ -1752,11 +1752,11 @@ update_eclasses(PlannerInfo *root, EquivalenceClass *ec, int from, int to)
 			j = -1;
 			while ((j = bms_next_member(rinfo->clause_relids, j)) >= 0)
 			{
-				RangeTblEntry *rte = root->simple_rte_array[j];
+				EquivalenceClassIndexes *index = &root->eclass_indexes_array[j];
 
-				Assert(bms_is_member(i, rte->eclass_source_indexes));
-				rte->eclass_source_indexes =
-					bms_del_member(rte->eclass_source_indexes, i);
+				Assert(bms_is_member(i, index->source_indexes));
+				index->source_indexes =
+					bms_del_member(index->source_indexes, i);
 			}
 
 			/*
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index a65a76ace9..3501fbbeed 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -483,13 +483,6 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
 	 */
 	childrte = makeNode(RangeTblEntry);
 	memcpy(childrte, parentrte, sizeof(RangeTblEntry));
-	/*
-	 * We do not want to inherit the EquivalenceMember indexes of the parent
-	 * to its child
-	 */
-	childrte->eclass_source_indexes = NULL;
-	childrte->eclass_derive_indexes = NULL;
-
 	Assert(parentrte->rtekind == RTE_RELATION); /* else this is dubious */
 	childrte->relid = childOID;
 	childrte->relkind = childrel->rd_rel->relkind;
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 9973326e90..bc9195c623 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -119,6 +119,9 @@ setup_simple_rel_arrays(PlannerInfo *root)
 		root->simple_rte_array[rti++] = rte;
 	}
 
+	root->eclass_indexes_array = (EquivalenceClassIndexes *)
+		palloc0(size * sizeof(EquivalenceClassIndexes));
+
 	/* append_rel_array is not needed if there are no AppendRelInfos */
 	if (root->append_rel_list == NIL)
 	{
@@ -217,6 +220,9 @@ expand_planner_arrays(PlannerInfo *root, int add_size)
 		root->top_parent_relid_array = NULL;
 	}
 
+	root->eclass_indexes_array =
+		repalloc0_array(root->eclass_indexes_array, EquivalenceClassIndexes, root->simple_rel_array_size, new_size);
+
 	root->simple_rel_array_size = new_size;
 }
 
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f624f41d99..baa6a97c7e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1194,12 +1194,6 @@ typedef struct RangeTblEntry
 	bool		inh;			/* inheritance requested? */
 	bool		inFromCl;		/* present in FROM clause? */
 	List	   *securityQuals;	/* security barrier quals to apply, if any */
-	Bitmapset  *eclass_source_indexes;	/* Indexes in PlannerInfo's eq_sources
-										 * list for RestrictInfos that mention
-										 * this relation */
-	Bitmapset  *eclass_derive_indexes;	/* Indexes in PlannerInfo's eq_derives
-										 * list for RestrictInfos that mention
-										 * this relation */
 } RangeTblEntry;
 
 /*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 81253d0351..49f53fd51e 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -189,6 +189,8 @@ typedef struct PlannerInfo PlannerInfo;
 #define HAVE_PLANNERINFO_TYPEDEF 1
 #endif
 
+struct EquivalenceClassIndexes;
+
 struct PlannerInfo
 {
 	pg_node_attr(no_copy_equal, no_read, no_query_jumble)
@@ -245,6 +247,13 @@ struct PlannerInfo
 	 */
 	struct AppendRelInfo **append_rel_array pg_node_attr(read_write_ignore);
 
+	/*
+	 * eclass_indexes_array is the same length as simple_rel_array and holds
+	 * the indexes of the corresponding rels for faster lookups of
+	 * RestrictInfo. See the EquivalenceClass comment for more details.
+	 */
+	struct EquivalenceClassIndexes *eclass_indexes_array pg_node_attr(read_write_ignore);
+
 	/*
 	 * append_rel_array is the same length as simple_rel_array and holds the
 	 * top-level parent indexes of the corresponding rels within
@@ -1509,6 +1518,21 @@ typedef struct
 	List	   *ec_members;	/* parent and child members*/
 } EquivalenceChildMemberIterator;
 
+/*
+ * EquivalenceClassIndexes
+ *
+ * As mentioned in the EquivalenceClass comment, we introduce a
+ * bitmapset-based indexing mechanism for faster lookups of RestrictInfo. This
+ * struct exists for each relation and holds the corresponding indexes.
+ */
+typedef struct EquivalenceClassIndexes
+{
+	Bitmapset  *source_indexes;	/* Indexes in PlannerInfo's eq_sources list for
+								   RestrictInfos that mention this relation */
+	Bitmapset  *derive_indexes;	/* Indexes in PlannerInfo's eq_derives list for
+								   RestrictInfos that mention this relation */
+} EquivalenceClassIndexes;
+
 /*
  * PathKeys
  *
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 8f86dd864e..565dce4040 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -663,6 +663,7 @@ EphemeralNamedRelationMetadata
 EphemeralNamedRelationMetadataData
 EquivalenceChildMemberIterator
 EquivalenceClass
+EquivalenceClassIndexes
 EquivalenceMember
 ErrorContextCallback
 ErrorData
-- 
2.42.0.windows.2