Hi All,

On Fri, Aug 11, 2023 at 6:24 PM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> Obtaining child clauses from parent clauses by translation and
> tracking the translations is less complex and may be more efficient
> too. I will post a patch on those lines soon.
>

PFA patch set to add infrastructure to track RestrictInfo translations
and reuse them.

PlannerInfo gets a new member "struct HTAB *child_rinfo_hash" which is
a hash table of hash tables keyed by RestrictInfo::rinfo_serial. Each
element in the array is a hash table of RestrictInfos keyed by
RestrictInfo::required_relids as explained in my previous email. When
building child clauses when a. building child join rels or b. when
reparameterizing paths, we first access the first level hash table
using RestrictInfo::rinfo_serial of the parent and search the required
translation by computing the child RestrictInfo::required_relids
obtained by translating RestrictInfo::required_relids of the parent
RestrictInfo. If the translation doesn't exist, we create one and add
to the hash table.

RestrictInfo::required_relids is same for a RestrictInfo and its
commuted RestrictInfo. The order of operands is important for
IndexClauses. Hence we track the commuted RestrictInfo in a new field
RestrictInfo::comm_rinfo. RestrictInfo::is_commuted differentiates
between a RestrictInfo and its commuted version. This is explained as
a comment in the patch. This scheme has a minor benefit of saving
memory when the same RestrictInfo is commuted multiple times.

Hash table of hash table is used instead of an array of hash tables
since a. not every rinfo_serial has a RestrictInfo associated with it
b. not every RestrictInfo has translations, c. I don't think the exact
size of this array is not known till the planning ends since we
continue to create new clauses as we create new RelOptInfos. Of
course, an array can be repalloc'ed and unused slots in the array may
not waste a lot of memory. I am open to change hash table to an array
which may be more efficient.

With these set of patches, the memory consumption stands as below
Number of tables | without patch  | with patch | % reduction |
being joined     |                |            |             |
--------------------------------------------------------------
               2 |      40.8 MiB  |   37.4 MiB |       8.46% |
               3 |     151.6 MiB  |  135.0 MiB |      10.96% |
               4 |     464.0 MiB  |  403.6 MiB |      12.00% |
               5 |    1663.9 MiB  | 1329.1 MiB |      20.12% |

The patch set is thus
0001 - patch used to measure memory used during planning

0002 - Patch to free intermediate Relids computed by
adjust_child_relids_multilevel(). I didn't test memory consumption for
multi-level partitioning. But this is clear improvement. In that
function we free the AppendInfos array which as many pointers long as
the number of relids. So it doesn't make sense not to free the Relids
which can be {largest relid}/8 bytes long at least.

0003 - patch to save and reuse commuted RestrictInfo. This patch by
itself shows a small memory saving (3%) in the query below where the
same clause is commuted twice. The query does not contain any
partitioned tables.
create table t2 (a int primary key, b int, c int);
create index t2_a_b on t2(a, b);
select * from t2 where 10 = a
Memory used without patch: 13696 bytes
Memory used with patch: 13264 bytes

0004 - Patch which implements the hash table of hash table described
above and also code to avoid repeated RestrictInfo list translations.

I will add this patchset to next commitfest.

--
Best Wishes,
Ashutosh Bapat
From 3b9e8039c15c87f6dc83369419c85c4a0a3b5688 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 31 Aug 2023 19:02:23 +0530
Subject: [PATCH 2/4] Free intermediate Relids created in
 adjust_child_relids_multilevel()

adjust_child_relids_multilevel() creates Relids for all the intermediate stages
in a partition hierarchy. These relid sets are not required finally. Relids or
Bitmapset take reasonably large space when thousands of partitions are
invovled. Hence free these intermediate relid sets.

Ashutosh Bapat
---
 src/backend/optimizer/util/appendinfo.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index f456b3b0a4..4008e52de2 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -591,6 +591,8 @@ adjust_child_relids_multilevel(PlannerInfo *root, Relids relids,
 {
 	AppendRelInfo **appinfos;
 	int			nappinfos;
+	Relids		tmp_relids = NULL;
+	Relids		child_relids;
 
 	/*
 	 * If the given relids set doesn't contain any of the parent relids, it
@@ -599,13 +601,14 @@ adjust_child_relids_multilevel(PlannerInfo *root, Relids relids,
 	if (!bms_overlap(relids, parentrel->relids))
 		return relids;
 
+	tmp_relids = relids;
 	/* Recurse if immediate parent is not the top parent. */
 	if (childrel->parent != parentrel)
 	{
 		if (childrel->parent)
-			relids = adjust_child_relids_multilevel(root, relids,
-													childrel->parent,
-													parentrel);
+			tmp_relids = adjust_child_relids_multilevel(root, relids,
+														childrel->parent,
+														parentrel);
 		else
 			elog(ERROR, "childrel is not a child of parentrel");
 	}
@@ -613,11 +616,15 @@ adjust_child_relids_multilevel(PlannerInfo *root, Relids relids,
 	/* Now translate for this child. */
 	appinfos = find_appinfos_by_relids(root, childrel->relids, &nappinfos);
 
-	relids = adjust_child_relids(relids, nappinfos, appinfos);
+	child_relids = adjust_child_relids(tmp_relids, nappinfos, appinfos);
+
+	/* Free any intermediate Relids created. */
+	if (tmp_relids != relids)
+		bms_free(tmp_relids);
 
 	pfree(appinfos);
 
-	return relids;
+	return child_relids;
 }
 
 /*
-- 
2.25.1

From 7eccbd4c61b37856c137577502dd1c9a4f36168a Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 12 Jul 2023 14:34:14 +0530
Subject: [PATCH 1/4] Report memory used for planning a query in EXPLAIN
 ANALYZE

The memory used in the CurrentMemoryContext and its children is sampled
before and after calling pg_plan_query() from ExplainOneQuery(). The
difference in the two samples is reported as the memory consumed while
planning the query. This may not account for the memory allocated in
memory contexts which are not children of CurrentMemoryContext. These
contexts are usually other long lived contexts, e.g.
CacheMemoryContext, which are shared by all the queries run in a
session. The consumption in those can not be attributed only to a given
query and hence should not be reported any way.

The memory consumption is reported as "Planning Memory" property in
EXPLAIN ANALYZE output.

Ashutosh Bapat
---
 src/backend/commands/explain.c        | 12 ++++++++++--
 src/backend/commands/prepare.c        |  7 ++++++-
 src/backend/utils/mmgr/mcxt.c         | 19 +++++++++++++++++++
 src/include/commands/explain.h        |  3 ++-
 src/include/utils/memutils.h          |  1 +
 src/test/regress/expected/explain.out | 15 +++++++++++----
 6 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 8570b14f62..7de2aa6bf0 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -397,16 +397,20 @@ ExplainOneQuery(Query *query, int cursorOptions,
 					planduration;
 		BufferUsage bufusage_start,
 					bufusage;
+		Size		mem_consumed;
 
 		if (es->buffers)
 			bufusage_start = pgBufferUsage;
 		INSTR_TIME_SET_CURRENT(planstart);
+		mem_consumed = MemoryContextMemUsed(CurrentMemoryContext);
 
 		/* plan the query */
 		plan = pg_plan_query(query, queryString, cursorOptions, params);
 
 		INSTR_TIME_SET_CURRENT(planduration);
 		INSTR_TIME_SUBTRACT(planduration, planstart);
+		mem_consumed = MemoryContextMemUsed(CurrentMemoryContext)
+			- mem_consumed;
 
 		/* calc differences of buffer counters. */
 		if (es->buffers)
@@ -417,7 +421,7 @@ ExplainOneQuery(Query *query, int cursorOptions,
 
 		/* run it (if needed) and produce output */
 		ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
-					   &planduration, (es->buffers ? &bufusage : NULL));
+					   &planduration, (es->buffers ? &bufusage : NULL), &mem_consumed);
 	}
 }
 
@@ -527,7 +531,7 @@ void
 ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
 			   const char *queryString, ParamListInfo params,
 			   QueryEnvironment *queryEnv, const instr_time *planduration,
-			   const BufferUsage *bufusage)
+			   const BufferUsage *bufusage, const Size *mem_consumed)
 {
 	DestReceiver *dest;
 	QueryDesc  *queryDesc;
@@ -628,6 +632,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
 		double		plantime = INSTR_TIME_GET_DOUBLE(*planduration);
 
 		ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
+
+		if (mem_consumed)
+			ExplainPropertyUInteger("Planning Memory", "bytes",
+									(uint64) *mem_consumed, es);
 	}
 
 	/* Print info about runtime of triggers */
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..02b48f845f 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -583,10 +583,12 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
 	instr_time	planduration;
 	BufferUsage bufusage_start,
 				bufusage;
+	Size		mem_consumed;
 
 	if (es->buffers)
 		bufusage_start = pgBufferUsage;
 	INSTR_TIME_SET_CURRENT(planstart);
+	mem_consumed = MemoryContextMemUsed(CurrentMemoryContext);
 
 	/* Look it up in the hash table */
 	entry = FetchPreparedStatement(execstmt->name, true);
@@ -623,6 +625,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
 
 	INSTR_TIME_SET_CURRENT(planduration);
 	INSTR_TIME_SUBTRACT(planduration, planstart);
+	mem_consumed = MemoryContextMemUsed(CurrentMemoryContext)
+		- mem_consumed;
 
 	/* calc differences of buffer counters. */
 	if (es->buffers)
@@ -640,7 +644,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
 
 		if (pstmt->commandType != CMD_UTILITY)
 			ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
-						   &planduration, (es->buffers ? &bufusage : NULL));
+						   &planduration, (es->buffers ? &bufusage : NULL),
+						   &mem_consumed);
 		else
 			ExplainOneUtility(pstmt->utilityStmt, into, es, query_string,
 							  paramLI, queryEnv);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 9fc83f11f6..43af271f33 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -747,6 +747,25 @@ MemoryContextStatsDetail(MemoryContext context, int max_children,
 								 grand_totals.totalspace - grand_totals.freespace)));
 }
 
+/*
+ * Compute the memory used in the given context and its children.
+ *
+ * XXX: Instead of this separate function we could modify
+ * MemoryContextStatsDetail() to report used memory and disable printing the
+ * detailed stats.
+ */
+extern Size
+MemoryContextMemUsed(MemoryContext context)
+{
+	MemoryContextCounters grand_totals;
+
+	memset(&grand_totals, 0, sizeof(grand_totals));
+
+	MemoryContextStatsInternal(context, 0, false, 100, &grand_totals, false);
+
+	return grand_totals.totalspace - grand_totals.freespace;
+}
+
 /*
  * MemoryContextStatsInternal
  *		One recursion level for MemoryContextStats
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 3d3e632a0c..21e3d2f309 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -92,7 +92,8 @@ extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
 						   ExplainState *es, const char *queryString,
 						   ParamListInfo params, QueryEnvironment *queryEnv,
 						   const instr_time *planduration,
-						   const BufferUsage *bufusage);
+						   const BufferUsage *bufusage,
+						   const Size *mem_consumed);
 
 extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc);
 extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 21640d62a6..d7c477f229 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -92,6 +92,7 @@ extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
 									 bool print_to_stderr);
 extern void MemoryContextAllowInCriticalSection(MemoryContext context,
 												bool allow);
+extern Size MemoryContextMemUsed(MemoryContext context);
 
 #ifdef MEMORY_CONTEXT_CHECKING
 extern void MemoryContextCheck(MemoryContext context);
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 1aca77491b..123ab22f5d 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -65,8 +65,9 @@ select explain_filter('explain (analyze) select * from int8_tbl i8');
 -----------------------------------------------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
  Planning Time: N.N ms
+ Planning Memory: N bytes
  Execution Time: N.N ms
-(3 rows)
+(4 rows)
 
 select explain_filter('explain (analyze, verbose) select * from int8_tbl i8');
                                             explain_filter                                            
@@ -74,16 +75,18 @@ select explain_filter('explain (analyze, verbose) select * from int8_tbl i8');
  Seq Scan on public.int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
    Output: q1, q2
  Planning Time: N.N ms
+ Planning Memory: N bytes
  Execution Time: N.N ms
-(4 rows)
+(5 rows)
 
 select explain_filter('explain (analyze, buffers, format text) select * from int8_tbl i8');
                                         explain_filter                                         
 -----------------------------------------------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
  Planning Time: N.N ms
+ Planning Memory: N bytes
  Execution Time: N.N ms
-(3 rows)
+(4 rows)
 
 select explain_filter('explain (analyze, buffers, format xml) select * from int8_tbl i8');
                      explain_filter                     
@@ -128,6 +131,7 @@ select explain_filter('explain (analyze, buffers, format xml) select * from int8
        <Temp-Written-Blocks>N</Temp-Written-Blocks>    +
      </Planning>                                       +
      <Planning-Time>N.N</Planning-Time>                +
+     <Planning-Memory>N</Planning-Memory>              +
      <Triggers>                                        +
      </Triggers>                                       +
      <Execution-Time>N.N</Execution-Time>              +
@@ -174,6 +178,7 @@ select explain_filter('explain (analyze, buffers, format yaml) select * from int
      Temp Read Blocks: N      +
      Temp Written Blocks: N   +
    Planning Time: N.N         +
+   Planning Memory: N         +
    Triggers:                  +
    Execution Time: N.N
 (1 row)
@@ -280,6 +285,7 @@ select explain_filter('explain (analyze, buffers, format json) select * from int
        "Temp I/O Write Time": N.N  +
      },                            +
      "Planning Time": N.N,         +
+     "Planning Memory": N,         +
      "Triggers": [                 +
      ],                            +
      "Execution Time": N.N         +
@@ -531,7 +537,8 @@ select jsonb_pretty(
          "Triggers": [                                      +
          ],                                                 +
          "Planning Time": 0.0,                              +
-         "Execution Time": 0.0                              +
+         "Execution Time": 0.0,                             +
+         "Planning Memory": 0                               +
      }                                                      +
  ]
 (1 row)
-- 
2.25.1

From 052cc695423560ca70ef296930019774bf6b659b Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 4 Sep 2023 14:56:21 +0530
Subject: [PATCH 3/4] Save commuted RestrictInfo for later use

A commuted RestrictInfo may be produced as many times as the number of indexes
it is used for. It's the same RestrictInfo always. Save some CPU and memory by
saving the result in the original RestrictInfo.

Ashutosh Bapat
---
 src/backend/optimizer/util/appendinfo.c   |  6 ++++++
 src/backend/optimizer/util/restrictinfo.c | 22 +++++++++++++++++++++-
 src/include/nodes/pathnodes.h             |  9 +++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index 4008e52de2..53e1233dce 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -492,6 +492,12 @@ adjust_appendrel_attrs_mutator(Node *node,
 		newinfo->left_mcvfreq = -1;
 		newinfo->right_mcvfreq = -1;
 
+		/*
+		 * Wipe out commuted parent RestrictInfo. The caller will compute
+		 * commuted clause if required.
+		 */
+		newinfo->comm_rinfo = NULL;
+
 		return (Node *) newinfo;
 	}
 
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index d6d26a2b51..9d67c623d0 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -246,6 +246,8 @@ make_restrictinfo_internal(PlannerInfo *root,
 
 	restrictinfo->left_hasheqoperator = InvalidOid;
 	restrictinfo->right_hasheqoperator = InvalidOid;
+	restrictinfo->comm_rinfo = NULL;
+	restrictinfo->is_commuted = false;
 
 	return restrictinfo;
 }
@@ -354,14 +356,27 @@ make_sub_restrictinfos(PlannerInfo *root,
  * be hazardous if the source is subject to change.  Also notice that we
  * assume without checking that the commutator op is a member of the same
  * btree and hash opclasses as the original op.
+ *
+ * If a commuted RestrictInfo is already available it is returned.
  */
 RestrictInfo *
 commute_restrictinfo(RestrictInfo *rinfo, Oid comm_op)
 {
 	RestrictInfo *result;
 	OpExpr	   *newclause;
-	OpExpr	   *clause = castNode(OpExpr, rinfo->clause);
+	OpExpr	   *clause;
+
+	if (rinfo->comm_rinfo)
+	{
+		result = rinfo->comm_rinfo;
+		newclause = castNode(OpExpr, result->clause);
+		Assert(list_length(newclause->args) == 2);
+		Assert(newclause->opno == comm_op);
 
+		return result;
+	}
+
+	clause = castNode(OpExpr, rinfo->clause);
 	Assert(list_length(clause->args) == 2);
 
 	/* flat-copy all the fields of clause ... */
@@ -403,6 +418,11 @@ commute_restrictinfo(RestrictInfo *rinfo, Oid comm_op)
 	result->right_mcvfreq = rinfo->left_mcvfreq;
 	result->left_hasheqoperator = InvalidOid;
 	result->right_hasheqoperator = InvalidOid;
+	result->is_commuted = !rinfo->is_commuted;
+	result->comm_rinfo = rinfo;
+
+	/* Save the commuted RestrictInfo for later use. */
+	rinfo->comm_rinfo = result;
 
 	return result;
 }
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 5702fbba60..c575fd11ec 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2517,6 +2517,13 @@ typedef struct LimitPath
  *
  * parent_ec, left_ec, right_ec are not printed, lest it lead to infinite
  * recursion in plan tree dump.
+ *
+ * A RestrictInfo may get commuted as many times as the number of indexes it is
+ * used for. The commuted clause is cached in the original RestrictInfo as
+ * comm_rinfo and vice versa. Both the RestrictInfos are commuted versions of
+ * each other. is_commuted flag is false for the first one to appear and is
+ * true in the other. The order doesn't matter. The flag just differentiate
+ * between the commuted version.
  */
 
 typedef struct RestrictInfo
@@ -2668,6 +2675,8 @@ typedef struct RestrictInfo
 	/* hash equality operators used for memoize nodes, else InvalidOid */
 	Oid			left_hasheqoperator pg_node_attr(equal_ignore);
 	Oid			right_hasheqoperator pg_node_attr(equal_ignore);
+	struct RestrictInfo *comm_rinfo pg_node_attr(equal_ignore);
+	bool		is_commuted pg_node_attr(equal_ignore);
 } RestrictInfo;
 
 /*
-- 
2.25.1

From 56f5ad9438aa6fe8a94d9b1033b8d99b9f440ba3 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 22 Jun 2023 14:27:14 +0530
Subject: [PATCH 4/4] Avoid translating RestrictInfo repeatedly

RestrictInfo to be applied to the child rels (including the child join rels)
are obtained by translating RestrictInfo applicable to the parent rel. Since
these translations are not tracked, the same RestrictInfo may get translated
multiple times for the same parent and child pair. When using partitionwise
join this can happen as many times as the number of possible join orders
between the partitioned tables or as many times a parent path is
reparameterized for a child if a clause is used in such a path.

Repeated translations are avoided by saving the translations in a hash
table. PlannerInfo maintains a hash table of hash tables to store and
retrieve translated RestrictInfos. The first level hash table is
referenced by RestrictInfo::rinfo_serial and the second level hash table
contains translated RestrictInfos referenced by
RestrictInfo::required_relids. Two function find_child_rinfo() and
add_child_rinfo() are used to search a RestrictInfo for a given
tanslation and add a child RestrictInfo respectively.

Translations of commuted clauses need further tracking.  When creating a
join clause using an equivalence class, if the child clause to be
created is commuted version of the parent clause mark the child clause
so and also add it to the hash table containing translations.  When
building a child clause from parent clause, commute the child clause if
the parent clause is commuted version.

Ashutosh Bapat
---
 src/backend/optimizer/path/equivclass.c   |  26 +++--
 src/backend/optimizer/path/joinrels.c     |  76 ++++++++++++-
 src/backend/optimizer/util/pathnode.c     | 123 ++++++++++++++++++++--
 src/backend/optimizer/util/relnode.c      |   7 +-
 src/backend/optimizer/util/restrictinfo.c | 123 ++++++++++++++++++++++
 src/include/nodes/pathnodes.h             |   4 +-
 src/include/optimizer/paths.h             |   2 +
 src/include/optimizer/restrictinfo.h      |   5 +
 src/tools/pgindent/typedefs.list          |   2 +
 9 files changed, 347 insertions(+), 21 deletions(-)

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 7fa502d6e2..4eb35088b8 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1816,7 +1816,7 @@ create_join_clause(PlannerInfo *root,
 				   EquivalenceMember *rightem,
 				   EquivalenceClass *parent_ec)
 {
-	RestrictInfo *rinfo;
+	RestrictInfo *rinfo = NULL;
 	RestrictInfo *parent_rinfo = NULL;
 	ListCell   *lc;
 	MemoryContext oldcontext;
@@ -1884,11 +1884,6 @@ create_join_clause(PlannerInfo *root,
 										bms_union(leftem->em_relids,
 												  rightem->em_relids),
 										ec->ec_min_security);
-
-	/* If it's a child clause, copy the parent's rinfo_serial */
-	if (parent_rinfo)
-		rinfo->rinfo_serial = parent_rinfo->rinfo_serial;
-
 	/* Mark the clause as redundant, or not */
 	rinfo->parent_ec = parent_ec;
 
@@ -1907,6 +1902,25 @@ create_join_clause(PlannerInfo *root,
 
 	MemoryContextSwitchTo(oldcontext);
 
+	/* We have obtained a translation through EC machinery, save it. */
+	if (parent_rinfo)
+	{
+		rinfo->rinfo_serial = parent_rinfo->rinfo_serial;
+
+		/*
+		 * If the child clause is commuted translation of the parent clause,
+		 * mark it accordingly.
+		 */
+		if ((leftem->em_parent &&
+			 leftem->em_parent == parent_rinfo->right_em) ||
+			(rightem->em_parent &&
+			 rightem->em_parent == parent_rinfo->left_em))
+			rinfo->is_commuted = !parent_rinfo->is_commuted;
+		else
+			rinfo->is_commuted = parent_rinfo->is_commuted;
+		add_child_rinfo(root, parent_rinfo, rinfo);
+	}
+
 	return rinfo;
 }
 
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index d03ace50a1..f991d38b77 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -19,6 +19,7 @@
 #include "optimizer/joininfo.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
+#include "optimizer/restrictinfo.h"
 #include "partitioning/partbounds.h"
 #include "utils/memutils.h"
 
@@ -1436,6 +1437,76 @@ restriction_is_constant_false(List *restrictlist,
 	return false;
 }
 
+/*
+ * Build the child RestrictInfo by translating the given parent RestrictInfo. If
+ * the translation is already available use it. Otherwise translate the parent
+ * clause and add to the available translations.
+ */
+static RestrictInfo *
+build_child_clause(PlannerInfo *root,
+				   RestrictInfo *parent_rinfo,
+				   int nappinfos, AppendRelInfo **appinfos)
+{
+	Relids		child_req_relids =
+		adjust_child_relids(parent_rinfo->required_relids,
+							nappinfos, appinfos);
+	RestrictInfo *child_rinfo;
+
+	/* If no child relids are involved no translation is required. */
+	if (bms_equal(child_req_relids, parent_rinfo->required_relids))
+		return parent_rinfo;
+
+	child_rinfo = find_child_rinfo(root, parent_rinfo, child_req_relids);
+	if (!child_rinfo)
+	{
+		child_rinfo = castNode(RestrictInfo,
+							   adjust_appendrel_attrs(root,
+													  (Node *) parent_rinfo,
+													  nappinfos,
+													  appinfos));
+		add_child_rinfo(root, parent_rinfo, child_rinfo);
+	}
+	bms_free(child_req_relids);
+
+	/* If the parent clause is commuted, return commuted child clause. */
+	if (parent_rinfo->is_commuted != child_rinfo->is_commuted)
+	{
+		if (!child_rinfo->comm_rinfo)
+		{
+			OpExpr	   *parent_clause = castNode(OpExpr, parent_rinfo->clause);
+
+			commute_restrictinfo(child_rinfo, parent_clause->opno);
+		}
+		child_rinfo = child_rinfo->comm_rinfo;
+	}
+
+	Assert(parent_rinfo->is_commuted == child_rinfo->is_commuted);
+	return child_rinfo;
+}
+
+/*
+ * Build the child RestrictInfo list by translating the given parent
+ * RestrictInfo list.
+ */
+List *
+build_child_clauses(PlannerInfo *root, List *parent_clauselist,
+					int nappinfos, AppendRelInfo **appinfos)
+{
+	ListCell   *lc;
+	List	   *child_clauselist = NIL;
+
+	foreach(lc, parent_clauselist)
+	{
+		RestrictInfo *parent_rinfo = lfirst_node(RestrictInfo, lc);
+		RestrictInfo *child_rinfo = build_child_clause(root, parent_rinfo,
+													   nappinfos, appinfos);
+
+		child_clauselist = lappend(child_clauselist, child_rinfo);
+	}
+
+	return child_clauselist;
+}
+
 /*
  * Assess whether join between given two partitioned relations can be broken
  * down into joins between matching partitions; a technique called
@@ -1631,9 +1702,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
 		 * applicable to the parent join.
 		 */
 		child_restrictlist =
-			(List *) adjust_appendrel_attrs(root,
-											(Node *) parent_restrictlist,
-											nappinfos, appinfos);
+			build_child_clauses(root, parent_restrictlist, nappinfos,
+								appinfos);
 
 		/* Find or construct the child join's RelOptInfo */
 		child_joinrel = joinrel->part_rels[cnt_parts];
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 211ba65389..7bddd36977 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -4024,6 +4024,89 @@ reparameterize_path(PlannerInfo *root, Path *path,
 	return NULL;
 }
 
+/*
+ * build_child_clauses_multilevel
+ *		Similar to build_child_clauses() but for translations through multiple levels of inheritance (actually partitioning) hierarchy.
+ *
+ * In a way this is similar to adjust_appendrel_attrs_multilevel() except that this function uses a translated RestrictInfo if it's available.
+ */
+static List *
+build_child_clauses_multilevel(PlannerInfo *root, List *parent_clauselist,
+							   RelOptInfo *child_rel,
+							   RelOptInfo *top_parent)
+{
+	AppendRelInfo **appinfos;
+	int			nappinfos;
+	List	   *tmp_clauselist = parent_clauselist;
+	List	   *child_clauselist;
+
+	if (child_rel->parent != top_parent)
+	{
+		if (child_rel->parent)
+		{
+			tmp_clauselist = build_child_clauses_multilevel(root,
+															parent_clauselist,
+															child_rel->parent,
+															top_parent);
+		}
+		else
+			elog(ERROR, "child_rel is not a child of top_parent");
+	}
+
+	appinfos = find_appinfos_by_relids(root, child_rel->relids, &nappinfos);
+	child_clauselist = build_child_clauses(root, tmp_clauselist,
+										   nappinfos, appinfos);
+	pfree(appinfos);
+	/* Build any intermediate clauselist this function built. */
+	if (tmp_clauselist != parent_clauselist)
+		list_free(tmp_clauselist);
+
+	return child_clauselist;
+}
+
+/*
+ * build_child_iclauses_multilevel
+ * 		Translate IndexClause list to be used for the
+ * given top parent relation for given child relation through the hierarchy of inheritance (actually partitioning).
+ *
+ * This is similar to adjust_appendrel_attrs_multilevel() for IndexClause except that it uses translated RestrictInfos if already available.
+ */
+static List *
+build_child_iclauses_multilevel(PlannerInfo *root,
+								List *parent_iclauselist,
+								RelOptInfo *childrel,
+								RelOptInfo *top_parent)
+{
+	ListCell   *lc;
+	List	   *child_iclauses = NIL;
+
+	foreach(lc, parent_iclauselist)
+	{
+		IndexClause *iclause = lfirst_node(IndexClause, lc);
+		List	   *rinfo_list = NIL;
+		List	   *child_rinfo_list;
+		IndexClause *child_iclause = makeNode(IndexClause);
+
+		/*
+		 * Collect the RestrictInfos to be translated. That's all what needs
+		 * to be translated right now.
+		 */
+		rinfo_list = lappend(rinfo_list, iclause->rinfo);
+		rinfo_list = list_concat(rinfo_list, iclause->indexquals);
+
+		child_rinfo_list = build_child_clauses_multilevel(root, rinfo_list,
+														  childrel, top_parent);
+		memcpy(child_iclause, iclause, sizeof(IndexClause));
+		child_iclause->rinfo = linitial(child_rinfo_list);
+		child_iclause->indexquals = list_delete_first(child_rinfo_list);
+
+		list_free(rinfo_list);
+		child_iclauses = lappend(child_iclauses, child_iclause);
+	}
+
+	return child_iclauses;
+}
+
 /*
  * reparameterize_path_by_child
  * 		Given a path parameterized by the parent of the given child relation,
@@ -4112,7 +4195,10 @@ do { \
 				IndexPath  *ipath;
 
 				FLAT_COPY_PATH(ipath, path, IndexPath);
-				ADJUST_CHILD_ATTRS(ipath->indexclauses);
+				ipath->indexclauses =
+					build_child_iclauses_multilevel(root,
+													ipath->indexclauses,
+													child_rel, child_rel->top_parent);
 				new_path = (Path *) ipath;
 			}
 			break;
@@ -4196,7 +4282,11 @@ do { \
 				jpath = (JoinPath *) npath;
 				REPARAMETERIZE_CHILD_PATH(jpath->outerjoinpath);
 				REPARAMETERIZE_CHILD_PATH(jpath->innerjoinpath);
-				ADJUST_CHILD_ATTRS(jpath->joinrestrictinfo);
+				jpath->joinrestrictinfo =
+					build_child_clauses_multilevel(root,
+												   jpath->joinrestrictinfo,
+												   child_rel,
+												   child_rel->top_parent);
 				new_path = (Path *) npath;
 			}
 			break;
@@ -4211,8 +4301,16 @@ do { \
 				jpath = (JoinPath *) mpath;
 				REPARAMETERIZE_CHILD_PATH(jpath->outerjoinpath);
 				REPARAMETERIZE_CHILD_PATH(jpath->innerjoinpath);
-				ADJUST_CHILD_ATTRS(jpath->joinrestrictinfo);
-				ADJUST_CHILD_ATTRS(mpath->path_mergeclauses);
+				jpath->joinrestrictinfo =
+					build_child_clauses_multilevel(root,
+												   jpath->joinrestrictinfo,
+												   child_rel,
+												   child_rel->top_parent);
+				mpath->path_mergeclauses =
+					build_child_clauses_multilevel(root,
+												   mpath->path_mergeclauses,
+												   child_rel,
+												   child_rel->top_parent);
 				new_path = (Path *) mpath;
 			}
 			break;
@@ -4227,8 +4325,16 @@ do { \
 				jpath = (JoinPath *) hpath;
 				REPARAMETERIZE_CHILD_PATH(jpath->outerjoinpath);
 				REPARAMETERIZE_CHILD_PATH(jpath->innerjoinpath);
-				ADJUST_CHILD_ATTRS(jpath->joinrestrictinfo);
-				ADJUST_CHILD_ATTRS(hpath->path_hashclauses);
+				jpath->joinrestrictinfo =
+					build_child_clauses_multilevel(root,
+												   jpath->joinrestrictinfo,
+												   child_rel,
+												   child_rel->top_parent);
+				hpath->path_hashclauses =
+					build_child_clauses_multilevel(root,
+												   hpath->path_hashclauses,
+												   child_rel,
+												   child_rel->top_parent);
 				new_path = (Path *) hpath;
 			}
 			break;
@@ -4310,7 +4416,10 @@ do { \
 		new_ppi->ppi_req_outer = bms_copy(required_outer);
 		new_ppi->ppi_rows = old_ppi->ppi_rows;
 		new_ppi->ppi_clauses = old_ppi->ppi_clauses;
-		ADJUST_CHILD_ATTRS(new_ppi->ppi_clauses);
+		new_ppi->ppi_clauses =
+			build_child_clauses_multilevel(root,
+										   new_ppi->ppi_clauses, child_rel,
+										   child_rel->top_parent);
 		new_ppi->ppi_serials = bms_copy(old_ppi->ppi_serials);
 		rel->ppilist = lappend(rel->ppilist, new_ppi);
 
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 76dad17e33..e69151d16f 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -949,10 +949,9 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 							   nappinfos, appinfos);
 
 	/* Construct joininfo list. */
-	joinrel->joininfo = (List *) adjust_appendrel_attrs(root,
-														(Node *) parent_joinrel->joininfo,
-														nappinfos,
-														appinfos);
+	joinrel->joininfo = build_child_clauses(root,
+											parent_joinrel->joininfo,
+											nappinfos, appinfos);
 
 	/*
 	 * Lateral relids referred in child join will be same as that referred in
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 9d67c623d0..ffb1692bb9 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -20,6 +20,19 @@
 #include "optimizer/optimizer.h"
 #include "optimizer/restrictinfo.h"
 
+typedef struct RinfoHashTabEntry
+{
+	int			rinfo_serial;	/* Key, must be the first one. */
+	HTAB	   *hash_tab;		/* Hash table containing child RestrictInfo
+								 * for RestrictInfo indicated by given
+								 * rinfo_serial. */
+} RinfoHashTabEntry;
+
+typedef struct ChildRinfoTabEntry
+{
+	Relids		required_relids;	/* Key, must be the first one */
+	RestrictInfo *child_rinfo;	/* Child RestrictInfo for required_relids. */
+} ChildRinfoTabEntry;
 
 static RestrictInfo *make_restrictinfo_internal(PlannerInfo *root,
 												Expr *clause,
@@ -42,6 +55,8 @@ static Expr *make_sub_restrictinfos(PlannerInfo *root,
 									Relids required_relids,
 									Relids incompatible_relids,
 									Relids outer_relids);
+static HTAB *get_child_rinfo_htab(PlannerInfo *root,
+								  RestrictInfo *parent_rinfo);
 
 
 /*
@@ -705,3 +720,111 @@ join_clause_is_movable_into(RestrictInfo *rinfo,
 
 	return true;
 }
+
+/*
+ * find_child_rinfo
+ *		Find the translation of the given parent RestrictInfo for the given
+ *		child relids.
+ *
+ * The given parent RestrictInfo need not be the top parent relations's
+ * RestrictInfo. We use RestrictInfo::rinfo_serial to access all the
+ * translations of the top parent relation's RestrictInfo.
+ *
+ * PlannerInfo maintains a hash table of hash tables to store and search
+ * translated RestrictInfos. The first level hash table is referenced by
+ * RestrictInfo::rinfo_serial and the second level hash table contains
+ * translated RestrictInfos referenced by RestrictInfo::required_relids.
+ */
+RestrictInfo *
+find_child_rinfo(PlannerInfo *root, RestrictInfo *parent_rinfo,
+				 Relids child_required_relids)
+{
+	HTAB	   *rinfo_hash = get_child_rinfo_htab(root, parent_rinfo);
+	ChildRinfoTabEntry *child_rinfo_entry;
+
+	child_rinfo_entry = hash_search(rinfo_hash, &child_required_relids,
+									HASH_FIND,
+									NULL);
+	return (child_rinfo_entry ? child_rinfo_entry->child_rinfo : NULL);
+}
+
+/*
+ * add_child_rinfo
+ *		Save the given child RestrictInfo in the hash table containing translations of the given parent RestrictInfo.
+ *
+ * The parent RestrictInfo need not be the top parent RestrictInfo. We use
+ * RestrictInfo::rinfo_serial to identify the set of RestrictInfos.
+ *
+ * The function assumes that the given child RestrictInfo is not already
+ * available in the hash table.
+ */
+extern void
+add_child_rinfo(PlannerInfo *root, RestrictInfo *parent_rinfo,
+				RestrictInfo *child_rinfo)
+{
+	HTAB	   *rinfo_hash = get_child_rinfo_htab(root, parent_rinfo);
+	ChildRinfoTabEntry *child_rinfo_entry;
+	bool		found;
+
+	Assert(child_rinfo->rinfo_serial == parent_rinfo->rinfo_serial);
+
+	child_rinfo_entry = hash_search(rinfo_hash, &(child_rinfo->required_relids),
+									HASH_ENTER, &found);
+
+	/*
+	 * A child restrictinfo should be translated only once and thus is
+	 * required to be added only once.
+	 */
+	Assert(!found);
+	child_rinfo_entry->child_rinfo = child_rinfo;
+}
+
+/*
+ * get_child_rinfo_htab
+ *		Given a parent RestrictInfo, return the hash table containing all its child RestrictInfos.
+ *
+ * A helper function to access the RestrictInfo translation hash tables. The function creates the required hash table if they are not already available.
+ */
+static HTAB *
+get_child_rinfo_htab(PlannerInfo *root, RestrictInfo *parent_rinfo)
+{
+	RinfoHashTabEntry *rinfo_tab_entry;
+	bool		found_htab;
+
+	if (!root->child_rinfo_hash)
+	{
+		HASHCTL		hash_ctl = {0};
+
+		Assert(sizeof(int) == sizeof(parent_rinfo->rinfo_serial));
+		hash_ctl.keysize = sizeof(int);
+		hash_ctl.entrysize = sizeof(RinfoHashTabEntry);
+		hash_ctl.hcxt = root->planner_cxt;
+
+		root->child_rinfo_hash = hash_create("hash of child rinfo hash",
+											 root->last_rinfo_serial,
+											 &hash_ctl,
+											 HASH_ELEM | HASH_BLOBS |
+											 HASH_CONTEXT);
+	}
+
+	rinfo_tab_entry = hash_search(root->child_rinfo_hash,
+								  &(parent_rinfo->rinfo_serial), HASH_ENTER,
+								  &found_htab);
+	if (!found_htab)
+	{
+		HASHCTL		hash_ctl = {0};
+
+		hash_ctl.keysize = sizeof(Relids);
+		hash_ctl.entrysize = sizeof(ChildRinfoTabEntry);
+		hash_ctl.hash = bitmap_hash;
+		hash_ctl.match = bitmap_match;
+		hash_ctl.hcxt = root->planner_cxt;
+
+		rinfo_tab_entry->hash_tab = hash_create("child rinfo hash", 100,
+												&hash_ctl,
+												HASH_ELEM | HASH_FUNCTION | HASH_COMPARE |
+												HASH_CONTEXT);
+	}
+
+	return rinfo_tab_entry->hash_tab;
+}
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index c575fd11ec..ffe4fc8ddc 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -338,6 +338,7 @@ struct PlannerInfo
 
 	/* counter for assigning RestrictInfo serial numbers */
 	int			last_rinfo_serial;
+	struct HTAB *child_rinfo_hash pg_node_attr(read_write_ignore);
 
 	/*
 	 * all_result_relids is empty for SELECT, otherwise it contains at least
@@ -2523,7 +2524,8 @@ typedef struct LimitPath
  * comm_rinfo and vice versa. Both the RestrictInfos are commuted versions of
  * each other. is_commuted flag is false for the first one to appear and is
  * true in the other. The order doesn't matter. The flag just differentiate
- * between the commuted version.
+ * between the commuted version. The child RestrictInfos inherit this flag from
+ * their respective parent RestrictInfo.
  */
 
 typedef struct RestrictInfo
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 50bc3b503a..02ab078188 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -112,6 +112,8 @@ extern bool have_join_order_restriction(PlannerInfo *root,
 extern bool have_dangerous_phv(PlannerInfo *root,
 							   Relids outer_relids, Relids inner_params);
 extern void mark_dummy_rel(RelOptInfo *rel);
+extern List *build_child_clauses(PlannerInfo *root, List *parent_clauselist,
+								 int nappinfos, AppendRelInfo **appinfos);
 
 /*
  * equivclass.c
diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h
index e140e619ac..507fa21e55 100644
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -47,5 +47,10 @@ extern bool join_clause_is_movable_to(RestrictInfo *rinfo, RelOptInfo *baserel);
 extern bool join_clause_is_movable_into(RestrictInfo *rinfo,
 										Relids currentrelids,
 										Relids current_and_outer);
+extern RestrictInfo *find_child_rinfo(PlannerInfo *root,
+									  RestrictInfo *parent_rinfo,
+									  Bitmapset *child_required_relids);
+extern void add_child_rinfo(PlannerInfo *root, RestrictInfo *parent_rinfo,
+							RestrictInfo *child_rinfo);
 
 #endif							/* RESTRICTINFO_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f2af84d7ca..ab713d5063 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -372,6 +372,7 @@ CheckPointStmt
 CheckpointStatsData
 CheckpointerRequest
 CheckpointerShmemStruct
+ChildRinfoTabEntry
 Chromosome
 CkptSortItem
 CkptTsStatus
@@ -2373,6 +2374,7 @@ RewriteMappingDataEntry
 RewriteMappingFile
 RewriteRule
 RewriteState
+RinfoHashTabEntry
 RmgrData
 RmgrDescData
 RmgrId
-- 
2.25.1

Reply via email to