On Thu, Sep 14, 2023 at 4:39 PM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
>
> 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
PFA rebased patches. Nothing changes in 0002, 0003 and 0004. 0001 is
the squashed version of the latest patch set at
https://www.postgresql.org/message-id/CAExHW5sCJX7696sF-OnugAiaXS=Ag95=-m1csrjcmyyj8pd...@mail.gmail.com.
--
Best Wishes,
Ashutosh Bapat
From de823e9cf0b03f21f8de51b49347df58ecb27c5a 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 69cfd8bd6ea38a99a715268423d92fa4179f7cc7 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 073eb3456c704a9b13bc53942b92ee64fbb3be5e 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
Helper patch which is squashed version of patches submitted in https://www.postgresql.org/message-id/CAExHW5sCJX7696sF-OnugAiaXS=Ag95=-m1csrjcmyyj8pd...@mail.gmail.com. Not to be committed with this patch set.
Ashutosh Bapat
---
src/backend/commands/explain.c | 79 ++++++++++++++++++++++++++-
src/backend/commands/prepare.c | 24 +++++++-
src/backend/utils/mmgr/mcxt.c | 12 ++++
src/include/commands/explain.h | 12 +++-
src/include/utils/memutils.h | 2 +
src/test/regress/expected/explain.out | 26 +++++++--
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 148 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 8570b14f62..e7df8e4455 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -122,6 +122,8 @@ static const char *explain_get_index_name(Oid indexId);
static void show_buffer_usage(ExplainState *es, const BufferUsage *usage,
bool planning);
static void show_wal_usage(ExplainState *es, const WalUsage *usage);
+static void show_planning_memory(ExplainState *es,
+ const MemUsage *usage);
static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
ExplainState *es);
static void ExplainScanTarget(Scan *plan, ExplainState *es);
@@ -397,9 +399,27 @@ ExplainOneQuery(Query *query, int cursorOptions,
planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts_start;
+ MemoryContextCounters mem_counts_end;
+ MemUsage mem_usage;
+ MemoryContext planner_ctx;
+ MemoryContext saved_ctx;
+
+ /*
+ * Create a new memory context to accurately measure memory malloc'ed
+ * by the planner. For further accuracy we should use the same type of
+ * memory context as the planner would use. That's usually AllocSet
+ * but ensure that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
if (es->buffers)
bufusage_start = pgBufferUsage;
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_start);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
INSTR_TIME_SET_CURRENT(planstart);
/* plan the query */
@@ -407,6 +427,9 @@ ExplainOneQuery(Query *query, int cursorOptions,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_end);
+ calc_mem_usage(&mem_usage, &mem_counts_end, &mem_counts_start);
/* calc differences of buffer counters. */
if (es->buffers)
@@ -417,7 +440,8 @@ 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_usage);
}
}
@@ -527,7 +551,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 MemUsage *mem_usage)
{
DestReceiver *dest;
QueryDesc *queryDesc;
@@ -630,6 +654,13 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
}
+ if (es->summary && mem_usage)
+ {
+ ExplainOpenGroup("Planning Memory", "Planning Memory", true, es);
+ show_planning_memory(es, mem_usage);
+ ExplainCloseGroup("Planning Memory", "Planning Memory", true, es);
+ }
+
/* Print info about runtime of triggers */
if (es->analyze)
ExplainPrintTriggers(es, queryDesc);
@@ -3736,6 +3767,50 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
}
}
+/*
+ * Show planner's memory usage details.
+ */
+static void
+show_planning_memory(ExplainState *es, const MemUsage *usage)
+{
+ if (es->format == EXPLAIN_FORMAT_TEXT)
+ {
+ appendStringInfo(es->str,
+ "Planning Memory: used=%zu bytes allocated=%zu bytes",
+ usage->mem_used, usage->mem_allocated);
+ appendStringInfoChar(es->str, '\n');
+ }
+ else
+ {
+ ExplainPropertyInteger("Used", "bytes", usage->mem_used, es);
+ ExplainPropertyInteger("Allocated", "bytes", usage->mem_allocated, es);
+ }
+}
+
+/*
+ * Compute memory usage from the start and end memory counts.
+ */
+void
+calc_mem_usage(MemUsage *mem_usage, MemoryContextCounters *mem_counts_end,
+ MemoryContextCounters *mem_counts_start)
+{
+ Size mem_used_start;
+ Size mem_used_end;
+
+ mem_used_start = mem_counts_start->totalspace - mem_counts_start->freespace;
+ mem_used_end = mem_counts_end->totalspace - mem_counts_end->freespace;
+
+ mem_usage->mem_used = mem_used_end - mem_used_start;
+
+ /*
+ * The net memory used is from total memory allocated and not necessarily
+ * the net memory allocated between the two given samples. Hence do not
+ * compute the difference between allocated memory reported in the two
+ * given samples.
+ */
+ mem_usage->mem_allocated = mem_counts_end->totalspace;
+}
+
/*
* Add some additional details about an IndexScan or IndexOnlyScan
*/
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..eb39823d7a 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -583,9 +583,27 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
instr_time planduration;
BufferUsage bufusage_start,
bufusage;
+ MemoryContextCounters mem_counts_start;
+ MemoryContextCounters mem_counts_end;
+ MemUsage mem_usage;
+ MemoryContext planner_ctx;
+ MemoryContext saved_ctx;
+
+ /*
+ * Create a new memory context to accurately measure memory malloc'ed by
+ * the planner. For further accuracy we should use the same type of memory
+ * context as the planner would use. That's usually AllocSet but ensure
+ * that.
+ */
+ Assert(IsA(CurrentMemoryContext, AllocSetContext));
+ planner_ctx = AllocSetContextCreate(CurrentMemoryContext,
+ "explain analyze planner context",
+ ALLOCSET_DEFAULT_SIZES);
if (es->buffers)
bufusage_start = pgBufferUsage;
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_start);
+ saved_ctx = MemoryContextSwitchTo(planner_ctx);
INSTR_TIME_SET_CURRENT(planstart);
/* Look it up in the hash table */
@@ -623,6 +641,9 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
INSTR_TIME_SET_CURRENT(planduration);
INSTR_TIME_SUBTRACT(planduration, planstart);
+ MemoryContextSwitchTo(saved_ctx);
+ MemoryContextMemConsumed(planner_ctx, &mem_counts_end);
+ calc_mem_usage(&mem_usage, &mem_counts_end, &mem_counts_start);
/* calc differences of buffer counters. */
if (es->buffers)
@@ -640,7 +661,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_usage);
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..94159a6799 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -747,6 +747,18 @@ MemoryContextStatsDetail(MemoryContext context, int max_children,
grand_totals.totalspace - grand_totals.freespace)));
}
+/*
+ * Return the memory used in the given context and its children.
+ */
+extern void
+MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *mem_consumed)
+{
+ memset(mem_consumed, 0, sizeof(*mem_consumed));
+
+ MemoryContextStatsInternal(context, 0, false, 100, mem_consumed, false);
+}
+
/*
* MemoryContextStatsInternal
* One recursion level for MemoryContextStats
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 3d3e632a0c..da2aeb647a 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -62,6 +62,12 @@ typedef struct ExplainState
ExplainWorkersState *workers_state; /* needed if parallel plan */
} ExplainState;
+typedef struct MemUsage
+{
+ Size mem_used;
+ Size mem_allocated;
+} MemUsage;
+
/* Hook for plugins to get control in ExplainOneQuery() */
typedef void (*ExplainOneQuery_hook_type) (Query *query,
int cursorOptions,
@@ -92,7 +98,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 MemUsage *mem_usage);
extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc);
extern void ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc);
@@ -125,5 +132,8 @@ extern void ExplainOpenGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es);
extern void ExplainCloseGroup(const char *objtype, const char *labelname,
bool labeled, ExplainState *es);
+extern void calc_mem_usage(MemUsage *mem_usage,
+ MemoryContextCounters *mem_counts_end,
+ MemoryContextCounters *mem_counts_start);
#endif /* EXPLAIN_H */
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 21640d62a6..d254a044c1 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -92,6 +92,8 @@ extern void MemoryContextStatsDetail(MemoryContext context, int max_children,
bool print_to_stderr);
extern void MemoryContextAllowInCriticalSection(MemoryContext context,
bool allow);
+extern void MemoryContextMemConsumed(MemoryContext context,
+ MemoryContextCounters *mem_consumed);
#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..863d3d79d2 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: used=N bytes allocated=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: used=N bytes allocated=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: used=N bytes allocated=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,10 @@ 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> +
+ <Used>N</Used> +
+ <Allocated>N</Allocated> +
+ </Planning-Memory> +
<Triggers> +
</Triggers> +
<Execution-Time>N.N</Execution-Time> +
@@ -174,6 +181,9 @@ 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: +
+ Used: N +
+ Allocated: N +
Triggers: +
Execution Time: N.N
(1 row)
@@ -280,6 +290,10 @@ select explain_filter('explain (analyze, buffers, format json) select * from int
"Temp I/O Write Time": N.N +
}, +
"Planning Time": N.N, +
+ "Planning Memory": { +
+ "Used": N, +
+ "Allocated": N +
+ }, +
"Triggers": [ +
], +
"Execution Time": N.N +
@@ -531,7 +545,11 @@ select jsonb_pretty(
"Triggers": [ +
], +
"Planning Time": 0.0, +
- "Execution Time": 0.0 +
+ "Execution Time": 0.0, +
+ "Planning Memory": { +
+ "Used": 0, +
+ "Allocated": 0 +
+ } +
} +
]
(1 row)
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f2af84d7ca..78ab04bef8 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1544,6 +1544,7 @@ MemoryContextData
MemoryContextMethodID
MemoryContextMethods
MemoryStatsPrintFunc
+MemUsage
MergeAction
MergeActionState
MergeAppend
--
2.25.1
From fb1bf7e6c603df7063904858c4a7d1cd1ca475d7 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 78ab04bef8..f4448b1250 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
@@ -2374,6 +2375,7 @@ RewriteMappingDataEntry
RewriteMappingFile
RewriteRule
RewriteState
+RinfoHashTabEntry
RmgrData
RmgrDescData
RmgrId
--
2.25.1
