Hi Amit,
On Fri, Mar 15, 2024 at 11:45 AM Amit Langote <amitlangot...@gmail.com>
wrote:
> > >
> > > That being said I'm a big fan of using a local variable on stack and
> > > filling it. I'd probably go with the usual palloc/pfree, because that
> > > makes it much easier to use - the callers would not be responsible for
> > > allocating the SpecialJoinInfo struct. Sure, it's a little bit of
> > > overhead, but with the AllocSet caching I doubt it's measurable.
> >
> > You are suggesting that instead of declaring a local variable of type
> > SpecialJoinInfo, build_child_join_sjinfo() should palloc() and return
> > SpecialJoinInfo which will be freed by free_child_sjinfo()
> > (free_child_sjinfo_members in the patch). I am fine with that.
>
> Agree with Tomas about using makeNode() / pfree(). Having the pfree()
> kind of makes it extra-evident that those SpecialJoinInfos are
> transitory.
>
Attached patch-set
0001 - original patch as is
0002 - addresses your first set of comments
0003 - uses palloc and pfree to allocate and deallocate SpecialJoinInfo
structure.
I will squash both 0002 and 0003 into 0001 once you review those changes
and are fine with those.
>
> > > I did put this through check-world on amd64/arm64, with valgrind,
> > > without any issue. I also tried the scripts shared by Ashutosh in his
> > > initial message (with some minor fixes, adding MEMORY to explain etc).
> > >
> > > The results with the 20240130 patches are like this:
> > >
> > > tables master patched
> > > -----------------------------
> > > 2 40.8 39.9
> > > 3 151.7 142.6
> > > 4 464.0 418.5
> > > 5 1663.9 1419.5
>
> Could you please post the numbers with the palloc() / pfree() version?
>
>
Here are they
tables | master | patched
--------+---------+---------
2 | 29 MB | 28 MB
3 | 102 MB | 93 MB
4 | 307 MB | 263 MB
5 | 1076 MB | 843 MB
The numbers look slightly different from my earlier numbers. But they were
quite old. The patch used to measure memory that time is different from the
one that we committed. So there's a slight difference in the way we measure
memory as well.
--
Best Wishes,
Ashutosh Bapat
From fc14dd22b1a150e82f99cad4440485daaf65eb00 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 27 Sep 2023 16:29:11 +0530
Subject: [PATCH 2/3] Address Amit's comments
---
src/backend/optimizer/path/joinrels.c | 13 +++++--------
src/include/nodes/pathnodes.h | 4 ++++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index d972aef988..05b3a6c017 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -1735,6 +1735,10 @@ build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
/*
* free_child_sjinfo_members
* Free memory consumed by members of 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)
@@ -1746,19 +1750,12 @@ free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo)
if (child_sjinfo->jointype == JOIN_INNER)
return;
- /*
- * The relids are used only for comparison and their references are not
- * stored anywhere. Free those.
- */
bms_free(child_sjinfo->min_lefthand);
bms_free(child_sjinfo->min_righthand);
bms_free(child_sjinfo->syn_lefthand);
bms_free(child_sjinfo->syn_righthand);
- /*
- * But the list of operator OIDs and the list of expressions may be
- * referenced somewhere else. Do not free those.
- */
+ /* semi_rhs_exprs may be referenced, so don't free. */
}
/*
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;
--
2.25.1
From 5a12690afe180defb8cc4f923fb0c4b89c2153a1 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 24 Jul 2023 20:20:53 +0530
Subject: [PATCH 1/3] 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.
Ashutosh Bapat
---
src/backend/optimizer/path/costsize.c | 37 +------
src/backend/optimizer/path/joinrels.c | 150 ++++++++++++++++++--------
src/include/optimizer/paths.h | 2 +
3 files changed, 108 insertions(+), 81 deletions(-)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 83a0aed051..390b818f69 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -5049,23 +5049,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,
@@ -5218,23 +5202,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..d972aef988 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -42,9 +42,12 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
List *parent_restrictlist);
-static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
- SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids);
+static void build_child_join_sjinfo(PlannerInfo *root,
+ SpecialJoinInfo *parent_sjinfo,
+ RelOptInfo *left_child,
+ RelOptInfo *right_child,
+ SpecialJoinInfo *child_sjinfo);
+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 +656,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 +745,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);
}
/*
@@ -1521,7 +1534,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
RelOptInfo *child_rel2;
bool rel1_empty;
bool rel2_empty;
- SpecialJoinInfo *child_sjinfo;
+ SpecialJoinInfo child_sjinfo;
List *child_restrictlist;
RelOptInfo *child_joinrel;
AppendRelInfo **appinfos;
@@ -1616,9 +1629,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);
+ build_child_join_sjinfo(root, parent_sjinfo, child_rel1,
+ child_rel2, &child_sjinfo);
/* Find the AppendRelInfo structures */
appinfos = find_appinfos_by_relids(root,
@@ -1641,7 +1653,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
{
child_joinrel = build_child_join_rel(root, child_rel1, child_rel2,
joinrel, child_restrictlist,
- child_sjinfo);
+ &child_sjinfo);
joinrel->part_rels[cnt_parts] = child_joinrel;
joinrel->live_parts = bms_add_member(joinrel->live_parts, cnt_parts);
joinrel->all_partrels = bms_add_members(joinrel->all_partrels,
@@ -1655,10 +1667,11 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
/* And make paths for the child join */
populate_joinrel_with_paths(root, child_rel1, child_rel2,
- child_joinrel, child_sjinfo,
+ child_joinrel, &child_sjinfo,
child_restrictlist);
pfree(appinfos);
+ free_child_sjinfo_members(&child_sjinfo);
}
}
@@ -1666,43 +1679,86 @@ 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 *
+static void
build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- Relids left_relids, Relids right_relids)
+ RelOptInfo *left_child, RelOptInfo *right_child,
+ SpecialJoinInfo *child_sjinfo)
{
- SpecialJoinInfo *sjinfo = makeNode(SpecialJoinInfo);
AppendRelInfo **left_appinfos;
int left_nappinfos;
AppendRelInfo **right_appinfos;
int right_nappinfos;
- 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;
+ }
+
+ 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;
+/*
+ * free_child_sjinfo_members
+ * Free memory consumed by members of a child SpecialJoinInfo.
+ */
+static void
+free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo)
+{
+ /*
+ * Dummy SpecialJoinInfos do not have any translated fields and hence have
+ * nothing to free.
+ */
+ if (child_sjinfo->jointype == JOIN_INNER)
+ return;
+
+ /*
+ * The relids are used only for comparison and their references are not
+ * stored anywhere. Free those.
+ */
+ bms_free(child_sjinfo->min_lefthand);
+ bms_free(child_sjinfo->min_righthand);
+ bms_free(child_sjinfo->syn_lefthand);
+ bms_free(child_sjinfo->syn_righthand);
+
+ /*
+ * But the list of operator OIDs and the list of expressions may be
+ * referenced somewhere else. Do not free those.
+ */
}
/*
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.25.1
From 26eea1ff25ac419c38f8a6d157acb37ebcf539b7 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Mon, 18 Mar 2024 14:38:39 +0530
Subject: [PATCH 3/3] Use palloc'ed SpecialJoinInfo instead of one on the stack
Per suggestion from Amit Langote and Tomas Vondra
---
src/backend/optimizer/path/joinrels.c | 56 +++++++++++++++------------
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 05b3a6c017..465dcf7d5d 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -42,12 +42,11 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
List *parent_restrictlist);
-static void build_child_join_sjinfo(PlannerInfo *root,
- SpecialJoinInfo *parent_sjinfo,
- RelOptInfo *left_child,
- RelOptInfo *right_child,
- SpecialJoinInfo *child_sjinfo);
-static void free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo);
+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);
static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
SpecialJoinInfo *parent_sjinfo,
@@ -1534,7 +1533,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
RelOptInfo *child_rel2;
bool rel1_empty;
bool rel2_empty;
- SpecialJoinInfo child_sjinfo;
+ SpecialJoinInfo *child_sjinfo;
List *child_restrictlist;
RelOptInfo *child_joinrel;
AppendRelInfo **appinfos;
@@ -1629,8 +1628,8 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* Construct SpecialJoinInfo from parent join relations's
* SpecialJoinInfo.
*/
- build_child_join_sjinfo(root, parent_sjinfo, child_rel1,
- child_rel2, &child_sjinfo);
+ child_sjinfo = build_child_join_sjinfo(root, parent_sjinfo, child_rel1,
+ child_rel2);
/* Find the AppendRelInfo structures */
appinfos = find_appinfos_by_relids(root,
@@ -1653,7 +1652,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
{
child_joinrel = build_child_join_rel(root, child_rel1, child_rel2,
joinrel, child_restrictlist,
- &child_sjinfo);
+ child_sjinfo);
joinrel->part_rels[cnt_parts] = child_joinrel;
joinrel->live_parts = bms_add_member(joinrel->live_parts, cnt_parts);
joinrel->all_partrels = bms_add_members(joinrel->all_partrels,
@@ -1667,7 +1666,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
/* And make paths for the child join */
populate_joinrel_with_paths(root, child_rel1, child_rel2,
- child_joinrel, &child_sjinfo,
+ child_joinrel, child_sjinfo,
child_restrictlist);
pfree(appinfos);
@@ -1683,22 +1682,22 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* If translations are added to or removed from this function, consider freeing
* the translated objects in free_child_sjinfo_members() appropriately.
*/
-static void
+static SpecialJoinInfo *
build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
- RelOptInfo *left_child, RelOptInfo *right_child,
- SpecialJoinInfo *child_sjinfo)
+ RelOptInfo *left_child, RelOptInfo *right_child)
{
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;
+ return child_sjinfo;
}
memcpy(child_sjinfo, parent_sjinfo, sizeof(SpecialJoinInfo));
@@ -1730,32 +1729,41 @@ build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
pfree(left_appinfos);
pfree(right_appinfos);
+
+ return child_sjinfo;
}
/*
* free_child_sjinfo_members
- * Free memory consumed by members of a child SpecialJoinInfo.
+ * 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)
+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)
- return;
+ 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. */
+ }
- bms_free(child_sjinfo->min_lefthand);
- bms_free(child_sjinfo->min_righthand);
- bms_free(child_sjinfo->syn_lefthand);
- bms_free(child_sjinfo->syn_righthand);
+ /* Avoid dangling pointer. */
+ *child_sjinfo_p = NULL;
- /* semi_rhs_exprs may be referenced, so don't free. */
+ pfree(child_sjinfo);
}
/*
--
2.25.1
