Hi Ashutosh,
On Tue, Mar 19, 2024 at 12:47 AM Ashutosh Bapat
<ashutosh.bapat....@gmail.com> wrote:
> On Mon, Mar 18, 2024 at 5:40 PM Amit Langote <amitlangot...@gmail.com> wrote:
>> >>
>> >> Sorry, I should’ve mentioned that I was interested in seeing cpu times to
>> >> compare the two approaches. Specifically, to see if the palloc / frees
>> >> add noticeable overhead.
>> >
>> > No problem. Here you go
>> >
>> > tables | master | patched | perc_change
>> > --------+----------+----------+-------------
>> > 2 | 477.87 | 492.32 | -3.02
>> > 3 | 1970.83 | 1989.52 | -0.95
>> > 4 | 6305.01 | 6268.81 | 0.57
>> > 5 | 19261.56 | 18684.86 | 2.99
>> >
>> > +ve change indicates reduced planning time. It seems that the planning
>> > time improves as the number of tables increases. But all the numbers are
>> > well within noise levels and thus may not show any improvement or
>> > deterioration. If you want, I can repeat the experiment.
>>
>> Thanks. I also wanted to see cpu time difference between the two
>> approaches of SpecialJoinInfo allocation -- on stack and on heap. So
>> comparing times with the previous version of the patch using stack
>> allocation and the new one with palloc. I'm hoping that the new
>> approach is only worse in the noise range.
>
>
> Ah, sorry, I didn't read it carefully. Alvaro made me realise that I have
> been gathering numbers using assert enabled builds, so they are not that
> reliable. Here they are with non-assert enabled builds.
>
> planning time (in ms) reported by EXPLAIN.
> tables | master | stack_alloc | perc_change_1 | palloc | perc_change_2 |
> total_perc_change
> --------+----------+-------------+---------------+----------+---------------+-------------------
> 2 | 338.1 | 333.92 | 1.24 | 332.16 | 0.53 |
> 1.76
> 3 | 1507.93 | 1475.76 | 2.13 | 1472.79 | 0.20 |
> 2.33
> 4 | 5099.45 | 4980.35 | 2.34 | 4947.3 | 0.66 |
> 2.98
> 5 | 15442.64 | 15531.94 | -0.58 | 15393.41 | 0.89 |
> 0.32
>
> stack_alloc = timings with SpecialJoinInfo on stack
> perc_change_1 = percentage change of above wrt master
> palloc = timings with palloc'ed SpecialJoinInfo
> perc_change_2 = percentage change of above wrt timings with stack_alloc
> total_perc_change = percentage change between master and all patches
>
> total change is within noise. Timing with palloc is better than that with
> SpecialJoinInfo on stack but only marginally. I don't think that means palloc
> based allocation is better or worse than stack based.
>
> You requested CPU time difference between stack based SpecialJoinInfo and
> palloc'ed SpecialJoinInfo. Here it is
> tables | stack_alloc | palloc | perc_change
> --------+-------------+----------+-------------
> 2 | 0.438204 | 0.438986 | -0.18
> 3 | 1.224672 | 1.238781 | -1.15
> 4 | 3.511317 | 3.663334 | -4.33
> 5 | 9.283856 | 9.340516 | -0.61
>
> Yes, there's a consistent degradation albeit within noise levels.
>
> The memory measurements
> tables | master | with_patch
> --------+--------+------------
> 2 | 26 MB | 26 MB
> 3 | 93 MB | 84 MB
> 4 | 277 MB | 235 MB
> 5 | 954 MB | 724 MB
>
> The first row shows the same value because of rounding. The actual values
> there are 27740432 and 26854704 resp.
>
> Please let me know if you need anything.
Thanks for repeating the benchmark. So I don't see a problem with
keeping the existing palloc() way of allocating the
SpecialJoinInfos(). We're adding a few cycles by adding
free_child_join_sjinfo() (see my delta patch about the rename), but
the reduction in memory consumption due to it, which is our goal here,
far outweighs what looks like a minor CPU slowdown.
I have squashed 0002, 0003 into 0001, done some changes of my own, and
attached the delta patch as 0002. I've listed the changes in the
commit message. Let me know if anything seems amiss.
I'd like to commit 0001 + 0002 + any other changes based on your
comments (if any) sometime early next week.
--
Thanks, Amit Langote
From 9d8c408b3379faab6269421f67d75530f1c4cde2 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 22 Mar 2024 16:51:48 +0900
Subject: [PATCH v1 2/2] Some fixes
* Revert (IMO) unnecessary modifications of build_child_join_sjinfo().
For example, hunks to rename sjinfo to child_sjinfo seem unnecessary,
because it's clear from the context that they are "child" sjinfos.
* Rename free_child_sjinfo_members() to free_child_join_sjinfo() to
be symmetric with build_child_join_sjinfo(). Note that the function
is charged with freeing the sjinfo itself too. Like
build_child_join_sjinfo(), name the argument sjinfo, not
child_sjinfo.
* Don't set the child sjinfo pointer to NULL after freeing it. While
a good convention in general, it's not really common in the planner
code, so doing it here seems a bit overkill
* Rename make_dummy_sjinfo() to init_dummy_sjinfo() because the
function is not really making it per se, only initializing fields
in the SpecialJoinInfo struct made available by the caller.
* Make init_dummy_sjinfo() take Relids instead of RelOptInfos because
that's what's needed. Also allows to reduce changes to
build_child_join_sjinfo()'s signature.
* Update the reason mentioned in a comment in free_child_join_sjinfo()
about why semi_rhs_expr is not freed -- it may be freed, but there's
no way to "deep" free it, so we leave it alone.
* Update a comment somewhere about why SpecialJoinInfo can be
"transient" sometimes.
---
src/backend/optimizer/path/costsize.c | 6 +-
src/backend/optimizer/path/joinrels.c | 125 +++++++++++++-------------
src/backend/optimizer/util/pathnode.c | 5 +-
src/include/optimizer/paths.h | 4 +-
4 files changed, 70 insertions(+), 70 deletions(-)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 1b76523e79..ee23ed7835 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5050,7 +5050,7 @@ compute_semi_anti_join_factors(PlannerInfo *root,
/*
* Also get the normal inner-join selectivity of the join clauses.
*/
- make_dummy_sjinfo(&norm_sjinfo, outerrel, innerrel);
+ init_dummy_sjinfo(&norm_sjinfo, outerrel->relids, innerrel->relids);
nselec = clauselist_selectivity(root,
joinquals,
@@ -5203,8 +5203,8 @@ approx_tuple_count(PlannerInfo *root, JoinPath *path, List *quals)
/*
* Make up a SpecialJoinInfo for JOIN_INNER semantics.
*/
- make_dummy_sjinfo(&sjinfo, path->outerjoinpath->parent,
- path->innerjoinpath->parent);
+ init_dummy_sjinfo(&sjinfo, path->outerjoinpath->parent->relids,
+ path->innerjoinpath->parent->relids);
/* Get the approximate selectivity */
foreach(l, quals)
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 465dcf7d5d..bef5b5f213 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -44,9 +44,8 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
List *parent_restrictlist);
static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
SpecialJoinInfo *parent_sjinfo,
- RelOptInfo *left_child,
- RelOptInfo *right_child);
-static void free_child_sjinfo_members(SpecialJoinInfo **child_sjinfo);
+ Relids left_relids, Relids right_relids);
+static void free_child_join_sjinfo(SpecialJoinInfo *child_sjinfo);
static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
@@ -656,18 +655,24 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
}
/*
- * make_dummy_sjinfo
- * Populate the given SpecialJoinInfo for a plain inner join, between rel1
- * and rel2, which does not have a SpecialJoinInfo associated with it.
+ * init_dummy_sjinfo
+ * Populate the given SpecialJoinInfo for a plain inner join between rel1
+ * and rel2
+ *
+ * Inner joins do not normally have an associated SpecialJoinInfo node. But
+ * some functions involved in join planning require one to be passed,
+ * containing at least the information of which relations are being joined,
+ * so we initialize that information here.
*/
void
-make_dummy_sjinfo(SpecialJoinInfo *sjinfo, RelOptInfo *rel1, RelOptInfo *rel2)
+init_dummy_sjinfo(SpecialJoinInfo *sjinfo, Relids left_relids,
+ Relids right_relids)
{
sjinfo->type = T_SpecialJoinInfo;
- sjinfo->min_lefthand = rel1->relids;
- sjinfo->min_righthand = rel2->relids;
- sjinfo->syn_lefthand = rel1->relids;
- sjinfo->syn_righthand = rel2->relids;
+ sjinfo->min_lefthand = left_relids;
+ sjinfo->min_righthand = right_relids;
+ sjinfo->syn_lefthand = left_relids;
+ sjinfo->syn_righthand = right_relids;
sjinfo->jointype = JOIN_INNER;
sjinfo->ojrelid = 0;
sjinfo->commute_above_l = NULL;
@@ -744,7 +749,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
if (sjinfo == NULL)
{
sjinfo = &sjinfo_data;
- make_dummy_sjinfo(sjinfo, rel1, rel2);
+ init_dummy_sjinfo(sjinfo, rel1->relids, rel2->relids);
}
/*
@@ -1628,8 +1633,9 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct SpecialJoinInfo from parent join relations's
* SpecialJoinInfo.
*/
- child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo, child_rel1,
- child_rel2);
+ child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo,
+ child_rel1->relids,
+ child_rel2->relids);
/* Find the AppendRelInfo structures */
appinfos = find_appinfos_by_relids(root,
@@ -1670,7 +1676,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
child_restrictlist);
pfree(appinfos);
- free_child_sjinfo_members(&child_sjinfo);
+ free_child_join_sjinfo(child_sjinfo);
}
}
@@ -1679,91 +1685,84 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* SpecialJoinInfo for the join between parents. left_relids and right_relids
* are the relids of left and right side of the join respectively.
*
- * If translations are added to or removed from this function, consider freeing
- * the translated objects in free_child_sjinfo_members() appropriately.
+ * If translations are added to or removed from this function, consider
+ * updating free_child_join_sjinfo() appropriately.
*/
static SpecialJoinInfo *
build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- RelOptInfo *left_child, RelOptInfo *right_child)
+ Relids left_relids, Relids right_relids)
{
+ SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
AppendRelInfo **left_appinfos;
int left_nappinfos;
AppendRelInfo **right_appinfos;
int right_nappinfos;
- SpecialJoinInfo *child_sjinfo = makeNode(SpecialJoinInfo);
/* Dummy SpecialJoinInfos can be created without any translation. */
if (parent_sjinfo->jointype == JOIN_INNER)
{
Assert(parent_sjinfo->ojrelid == 0);
- make_dummy_sjinfo(child_sjinfo, left_child, right_child);
- return child_sjinfo;
+ init_dummy_sjinfo(sjinfo, left_relids, right_relids);
+ return sjinfo;
}
- memcpy(child_sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
- left_appinfos = find_appinfos_by_relids(root, left_child->relids,
+ memcpy(sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
+ left_appinfos = find_appinfos_by_relids(root, left_relids,
&left_nappinfos);
- right_appinfos = find_appinfos_by_relids(root, right_child->relids,
+ right_appinfos = find_appinfos_by_relids(root, right_relids,
&right_nappinfos);
- child_sjinfo->min_lefthand =
- adjust_child_relids(child_sjinfo->min_lefthand,
- left_nappinfos,
- left_appinfos);
- child_sjinfo->min_righthand =
- adjust_child_relids(child_sjinfo->min_righthand,
- right_nappinfos,
- right_appinfos);
- child_sjinfo->syn_lefthand = adjust_child_relids(child_sjinfo->syn_lefthand,
- left_nappinfos,
- left_appinfos);
- child_sjinfo->syn_righthand = adjust_child_relids(child_sjinfo->syn_righthand,
- right_nappinfos,
- right_appinfos);
-
+ sjinfo->min_lefthand = adjust_child_relids(sjinfo->min_lefthand,
+ left_nappinfos, left_appinfos);
+ sjinfo->min_righthand = adjust_child_relids(sjinfo->min_righthand,
+ right_nappinfos,
+ right_appinfos);
+ sjinfo->syn_lefthand = adjust_child_relids(sjinfo->syn_lefthand,
+ left_nappinfos, left_appinfos);
+ sjinfo->syn_righthand = adjust_child_relids(sjinfo->syn_righthand,
+ right_nappinfos,
+ right_appinfos);
/* outer-join relids need no adjustment */
- child_sjinfo->semi_rhs_exprs =
- (List *) adjust_appendrel_attrs(root,
- (Node *) child_sjinfo->semi_rhs_exprs,
- right_nappinfos, right_appinfos);
+ sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
+ (Node *) sjinfo->semi_rhs_exprs,
+ right_nappinfos,
+ right_appinfos);
pfree(left_appinfos);
pfree(right_appinfos);
- return child_sjinfo;
+ return sjinfo;
}
/*
- * free_child_sjinfo_members
- * Free memory consumed by a child SpecialJoinInfo.
+ * free_child_join_sjinfo
+ * Free memory consumed by a SpecialJoinInfo created by
+ * build_child_join_sjinfo()
*
* Only members that are translated copies of their counterpart in the parent
- * SpecialJoinInfo are freed here. However, members that could be referenced
- * elsewhere are not freed.
+ * SpecialJoinInfo are freed here; see build_child_join_sjinfo().
*/
static void
-free_child_sjinfo_members(SpecialJoinInfo **child_sjinfo_p)
+free_child_join_sjinfo(SpecialJoinInfo *sjinfo)
{
- SpecialJoinInfo *child_sjinfo = *child_sjinfo_p;
-
/*
- * Dummy SpecialJoinInfos do not have any translated fields and hence have
- * nothing to free.
+ * Dummy SpecialJoinInfos of inner joins do not have any translated fields
+ * and hence have nothing to be freed; see init_dummy_sjinfo().
*/
- if (child_sjinfo->jointype != JOIN_INNER)
+ if (sjinfo->jointype != JOIN_INNER)
{
- bms_free(child_sjinfo->min_lefthand);
- bms_free(child_sjinfo->min_righthand);
- bms_free(child_sjinfo->syn_lefthand);
- bms_free(child_sjinfo->syn_righthand);
+ bms_free(sjinfo->min_lefthand);
+ bms_free(sjinfo->min_righthand);
+ bms_free(sjinfo->syn_lefthand);
+ bms_free(sjinfo->syn_righthand);
- /* semi_rhs_exprs may be referenced, so don't free. */
+ /*
+ * semi_rhs_exprs may in principle be freed, but a simple pfree() does
+ * not suffice, so we leave it alone.
+ */
}
- /* Avoid dangling pointer. */
- *child_sjinfo_p = NULL;
-
- pfree(child_sjinfo);
+ pfree(sjinfo);
}
/*
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 0a7e5c2678..c29ca5a0da 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1707,8 +1707,9 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
pathnode->subpath = subpath;
/*
- * Under GEQO, the sjinfo might be short-lived, so we'd better make copies
- * of data structures we extract from it.
+ * Under GEQO and when planning child joins, the sjinfo might be
+ * short-lived, so we'd better make copies of data structures we extract
+ * from it.
*/
pathnode->in_operators = copyObject(sjinfo->semi_operators);
pathnode->uniq_exprs = copyObject(sjinfo->semi_rhs_exprs);
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 1e340a6335..125676b5bf 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -112,8 +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 void make_dummy_sjinfo(SpecialJoinInfo *sjinfo, RelOptInfo *rel1,
- RelOptInfo *rel2);
+extern void init_dummy_sjinfo(SpecialJoinInfo *sjinfo, Relids left_relids,
+ Relids right_relids);
/*
* equivclass.c
--
2.43.0
From 921dd02c65981b8e08db390a94b827769e8385b0 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 22 Mar 2024 14:07:48 +0900
Subject: [PATCH v1 1/2] Reduce memory used by child SpecialJoinInfo
The SpecialJoinInfo applicable to a child join is computed by
translating the same applicable to the parent join in
try_partitionwise_join(). The child SpecialJoinInfo is not needed once
the child join RelOptInfo is created and paths are added to it. Use a
local variable to hold child SpecialJoinInfo so that it doesn't need
memory allocated separately.
Also free the memory allocated to various Bitmapsets in SpecialJoinInfo.
Those are not referenced anywhere. But we do not free the memory
allocated to expression trees since those may be referenced in paths or
other objects.
Plain inner joins do not have SpecialJoinInfos associated with them.
They are crafted on the fly for parent joins. Do the same for child
joins as well.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://postgr.es/m/CAExHW5tHqEf3ASVqvFFcghYGPfpy7o3xnvhHwBGbJFMRH8KjNw@mail.gmail.com
---
src/backend/optimizer/path/costsize.c | 37 +------
src/backend/optimizer/path/joinrels.c | 143 ++++++++++++++++++--------
src/include/nodes/pathnodes.h | 4 +
src/include/optimizer/paths.h | 2 +
4 files changed, 111 insertions(+), 75 deletions(-)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 3c14c605a0..1b76523e79 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5050,23 +5050,7 @@ compute_semi_anti_join_factors(PlannerInfo *root,
/*
* Also get the normal inner-join selectivity of the join clauses.
*/
- norm_sjinfo.type = T_SpecialJoinInfo;
- norm_sjinfo.min_lefthand = outerrel->relids;
- norm_sjinfo.min_righthand = innerrel->relids;
- norm_sjinfo.syn_lefthand = outerrel->relids;
- norm_sjinfo.syn_righthand = innerrel->relids;
- norm_sjinfo.jointype = JOIN_INNER;
- norm_sjinfo.ojrelid = 0;
- norm_sjinfo.commute_above_l = NULL;
- norm_sjinfo.commute_above_r = NULL;
- norm_sjinfo.commute_below_l = NULL;
- norm_sjinfo.commute_below_r = NULL;
- /* we don't bother trying to make the remaining fields valid */
- norm_sjinfo.lhs_strict = false;
- norm_sjinfo.semi_can_btree = false;
- norm_sjinfo.semi_can_hash = false;
- norm_sjinfo.semi_operators = NIL;
- norm_sjinfo.semi_rhs_exprs = NIL;
+ make_dummy_sjinfo(&norm_sjinfo, outerrel, innerrel);
nselec = clauselist_selectivity(root,
joinquals,
@@ -5219,23 +5203,8 @@ approx_tuple_count(PlannerInfo *root, JoinPath *path, List *quals)
/*
* Make up a SpecialJoinInfo for JOIN_INNER semantics.
*/
- sjinfo.type = T_SpecialJoinInfo;
- sjinfo.min_lefthand = path->outerjoinpath->parent->relids;
- sjinfo.min_righthand = path->innerjoinpath->parent->relids;
- sjinfo.syn_lefthand = path->outerjoinpath->parent->relids;
- sjinfo.syn_righthand = path->innerjoinpath->parent->relids;
- sjinfo.jointype = JOIN_INNER;
- sjinfo.ojrelid = 0;
- sjinfo.commute_above_l = NULL;
- sjinfo.commute_above_r = NULL;
- sjinfo.commute_below_l = NULL;
- sjinfo.commute_below_r = NULL;
- /* we don't bother trying to make the remaining fields valid */
- sjinfo.lhs_strict = false;
- sjinfo.semi_can_btree = false;
- sjinfo.semi_can_hash = false;
- sjinfo.semi_operators = NIL;
- sjinfo.semi_rhs_exprs = NIL;
+ make_dummy_sjinfo(&sjinfo, path->outerjoinpath->parent,
+ path->innerjoinpath->parent);
/* Get the approximate selectivity */
foreach(l, quals)
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 4750579b0a..465dcf7d5d 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -44,7 +44,9 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
List *parent_restrictlist);
static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids);
+ RelOptInfo *left_child,
+ RelOptInfo *right_child);
+static void free_child_sjinfo_members(SpecialJoinInfo **child_sjinfo);
static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
@@ -653,6 +655,32 @@ join_is_legal(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
return true;
}
+/*
+ * make_dummy_sjinfo
+ * Populate the given SpecialJoinInfo for a plain inner join, between rel1
+ * and rel2, which does not have a SpecialJoinInfo associated with it.
+ */
+void
+make_dummy_sjinfo(SpecialJoinInfo *sjinfo, RelOptInfo *rel1, RelOptInfo *rel2)
+{
+ sjinfo->type = T_SpecialJoinInfo;
+ sjinfo->min_lefthand = rel1->relids;
+ sjinfo->min_righthand = rel2->relids;
+ sjinfo->syn_lefthand = rel1->relids;
+ sjinfo->syn_righthand = rel2->relids;
+ sjinfo->jointype = JOIN_INNER;
+ sjinfo->ojrelid = 0;
+ sjinfo->commute_above_l = NULL;
+ sjinfo->commute_above_r = NULL;
+ sjinfo->commute_below_l = NULL;
+ sjinfo->commute_below_r = NULL;
+ /* we don't bother trying to make the remaining fields valid */
+ sjinfo->lhs_strict = false;
+ sjinfo->semi_can_btree = false;
+ sjinfo->semi_can_hash = false;
+ sjinfo->semi_operators = NIL;
+ sjinfo->semi_rhs_exprs = NIL;
+}
/*
* make_join_rel
@@ -716,23 +744,7 @@ make_join_rel(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
if (sjinfo == NULL)
{
sjinfo = &sjinfo_data;
- sjinfo->type = T_SpecialJoinInfo;
- sjinfo->min_lefthand = rel1->relids;
- sjinfo->min_righthand = rel2->relids;
- sjinfo->syn_lefthand = rel1->relids;
- sjinfo->syn_righthand = rel2->relids;
- sjinfo->jointype = JOIN_INNER;
- sjinfo->ojrelid = 0;
- sjinfo->commute_above_l = NULL;
- sjinfo->commute_above_r = NULL;
- sjinfo->commute_below_l = NULL;
- sjinfo->commute_below_r = NULL;
- /* we don't bother trying to make the remaining fields valid */
- sjinfo->lhs_strict = false;
- sjinfo->semi_can_btree = false;
- sjinfo->semi_can_hash = false;
- sjinfo->semi_operators = NIL;
- sjinfo->semi_rhs_exprs = NIL;
+ make_dummy_sjinfo(sjinfo, rel1, rel2);
}
/*
@@ -1616,9 +1628,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct SpecialJoinInfo from parent join relations's
* SpecialJoinInfo.
*/
- child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo,
- child_rel1->relids,
- child_rel2->relids);
+ child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo, child_rel1,
+ child_rel2);
/* Find the AppendRelInfo structures */
appinfos = find_appinfos_by_relids(root,
@@ -1659,6 +1670,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
child_restrictlist);
pfree(appinfos);
+ free_child_sjinfo_members(&child_sjinfo);
}
}
@@ -1666,43 +1678,92 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct the SpecialJoinInfo for a child-join by translating
* SpecialJoinInfo for the join between parents. left_relids and right_relids
* are the relids of left and right side of the join respectively.
+ *
+ * If translations are added to or removed from this function, consider freeing
+ * the translated objects in free_child_sjinfo_members() appropriately.
*/
static SpecialJoinInfo *
build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids)
+ RelOptInfo *left_child, RelOptInfo *right_child)
{
- SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
AppendRelInfo **left_appinfos;
int left_nappinfos;
AppendRelInfo **right_appinfos;
int right_nappinfos;
+ SpecialJoinInfo *child_sjinfo = makeNode(SpecialJoinInfo);
- memcpy(sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
- left_appinfos = find_appinfos_by_relids(root, left_relids,
+ /* Dummy SpecialJoinInfos can be created without any translation. */
+ if (parent_sjinfo->jointype == JOIN_INNER)
+ {
+ Assert(parent_sjinfo->ojrelid == 0);
+ make_dummy_sjinfo(child_sjinfo, left_child, right_child);
+ return child_sjinfo;
+ }
+
+ memcpy(child_sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
+ left_appinfos = find_appinfos_by_relids(root, left_child->relids,
&left_nappinfos);
- right_appinfos = find_appinfos_by_relids(root, right_relids,
+ right_appinfos = find_appinfos_by_relids(root, right_child->relids,
&right_nappinfos);
- sjinfo->min_lefthand = adjust_child_relids(sjinfo->min_lefthand,
- left_nappinfos, left_appinfos);
- sjinfo->min_righthand = adjust_child_relids(sjinfo->min_righthand,
- right_nappinfos,
- right_appinfos);
- sjinfo->syn_lefthand = adjust_child_relids(sjinfo->syn_lefthand,
- left_nappinfos, left_appinfos);
- sjinfo->syn_righthand = adjust_child_relids(sjinfo->syn_righthand,
- right_nappinfos,
- right_appinfos);
+ child_sjinfo->min_lefthand =
+ adjust_child_relids(child_sjinfo->min_lefthand,
+ left_nappinfos,
+ left_appinfos);
+ child_sjinfo->min_righthand =
+ adjust_child_relids(child_sjinfo->min_righthand,
+ right_nappinfos,
+ right_appinfos);
+ child_sjinfo->syn_lefthand = adjust_child_relids(child_sjinfo->syn_lefthand,
+ left_nappinfos,
+ left_appinfos);
+ child_sjinfo->syn_righthand = adjust_child_relids(child_sjinfo->syn_righthand,
+ right_nappinfos,
+ right_appinfos);
+
/* outer-join relids need no adjustment */
- sjinfo->semi_rhs_exprs = (List *) adjust_appendrel_attrs(root,
- (Node *) sjinfo->semi_rhs_exprs,
- right_nappinfos,
- right_appinfos);
+ child_sjinfo->semi_rhs_exprs =
+ (List *) adjust_appendrel_attrs(root,
+ (Node *) child_sjinfo->semi_rhs_exprs,
+ right_nappinfos, right_appinfos);
pfree(left_appinfos);
pfree(right_appinfos);
- return sjinfo;
+ return child_sjinfo;
+}
+
+/*
+ * free_child_sjinfo_members
+ * Free memory consumed by a child SpecialJoinInfo.
+ *
+ * Only members that are translated copies of their counterpart in the parent
+ * SpecialJoinInfo are freed here. However, members that could be referenced
+ * elsewhere are not freed.
+ */
+static void
+free_child_sjinfo_members(SpecialJoinInfo **child_sjinfo_p)
+{
+ SpecialJoinInfo *child_sjinfo = *child_sjinfo_p;
+
+ /*
+ * Dummy SpecialJoinInfos do not have any translated fields and hence have
+ * nothing to free.
+ */
+ if (child_sjinfo->jointype != JOIN_INNER)
+ {
+ bms_free(child_sjinfo->min_lefthand);
+ bms_free(child_sjinfo->min_righthand);
+ bms_free(child_sjinfo->syn_lefthand);
+ bms_free(child_sjinfo->syn_righthand);
+
+ /* semi_rhs_exprs may be referenced, so don't free. */
+ }
+
+ /* Avoid dangling pointer. */
+ *child_sjinfo_p = NULL;
+
+ pfree(child_sjinfo);
}
/*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 534692bee1..7192ae5171 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2854,6 +2854,10 @@ typedef struct PlaceHolderVar
* cost estimation purposes it is sometimes useful to know the join size under
* plain innerjoin semantics. Note that lhs_strict and the semi_xxx fields
* are not set meaningfully within such structs.
+ *
+ * We also create transient SpecialJoinInfos for child joins by translating
+ * corresponding parent SpecialJoinInfos. These are also not part of
+ * PlannerInfo's join_info_list.
*/
#ifndef HAVE_SPECIALJOININFO_TYPEDEF
typedef struct SpecialJoinInfo SpecialJoinInfo;
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index b137c8a589..1e340a6335 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 void make_dummy_sjinfo(SpecialJoinInfo *sjinfo, RelOptInfo *rel1,
+ RelOptInfo *rel2);
/*
* equivclass.c
--
2.43.0
