Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c39392e

Browse files
author
Richard Guo
committed
Fix freeing a child join's SpecialJoinInfo
In try_partitionwise_join, we try to break down the join between two partitioned relations into joins between matching partitions. To achieve this, we iterate through each pair of partitions from the two joining relations and create child join relations for them. To reduce memory accumulation during each iteration, one step we take is freeing the SpecialJoinInfos created for the child joins. A child join's SpecialJoinInfo is a copy of the parent join's SpecialJoinInfo, with some members being translated copies of their counterparts in the parent. However, when freeing the bitmapset members in a child join's SpecialJoinInfo, we failed to check whether they were translated copies. As a result, we inadvertently freed the members that were still in use by the parent SpecialJoinInfo, leading to crashes when those freed members were accessed. To fix, check if each member of the child join's SpecialJoinInfo is a translated copy and free it only if that's the case. This requires passing the parent join's SpecialJoinInfo as a parameter to free_child_join_sjinfo. Back-patch to v17 where this bug crept in. Bug: #18806 Reported-by: 孟令彬 <m_lingbin@126.com> Diagnosed-by: Tender Wang <tndrwang@gmail.com> Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/18806-d70b0c9fdf63dcbf@postgresql.org Backpatch-through: 17
1 parent aef6f90 commit c39392e

File tree

3 files changed

+64
-9
lines changed

3 files changed

+64
-9
lines changed

src/backend/optimizer/path/joinrels.c

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1,
4545
static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root,
4646
SpecialJoinInfo *parent_sjinfo,
4747
Relids left_relids, Relids right_relids);
48-
static void free_child_join_sjinfo(SpecialJoinInfo *sjinfo);
48+
static void free_child_join_sjinfo(SpecialJoinInfo *child_sjinfo,
49+
SpecialJoinInfo *parent_sjinfo);
4950
static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1,
5051
RelOptInfo *rel2, RelOptInfo *joinrel,
5152
SpecialJoinInfo *parent_sjinfo,
@@ -1687,7 +1688,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
16871688
*/
16881689
pfree(appinfos);
16891690
bms_free(child_relids);
1690-
free_child_join_sjinfo(child_sjinfo);
1691+
free_child_join_sjinfo(child_sjinfo, parent_sjinfo);
16911692
}
16921693
}
16931694

@@ -1754,26 +1755,41 @@ build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo,
17541755
* SpecialJoinInfo are freed here.
17551756
*/
17561757
static void
1757-
free_child_join_sjinfo(SpecialJoinInfo *sjinfo)
1758+
free_child_join_sjinfo(SpecialJoinInfo *child_sjinfo,
1759+
SpecialJoinInfo *parent_sjinfo)
17581760
{
17591761
/*
17601762
* Dummy SpecialJoinInfos of inner joins do not have any translated fields
17611763
* and hence no fields that to be freed.
17621764
*/
1763-
if (sjinfo->jointype != JOIN_INNER)
1765+
if (child_sjinfo->jointype != JOIN_INNER)
17641766
{
1765-
bms_free(sjinfo->min_lefthand);
1766-
bms_free(sjinfo->min_righthand);
1767-
bms_free(sjinfo->syn_lefthand);
1768-
bms_free(sjinfo->syn_righthand);
1767+
if (child_sjinfo->min_lefthand != parent_sjinfo->min_lefthand)
1768+
bms_free(child_sjinfo->min_lefthand);
1769+
1770+
if (child_sjinfo->min_righthand != parent_sjinfo->min_righthand)
1771+
bms_free(child_sjinfo->min_righthand);
1772+
1773+
if (child_sjinfo->syn_lefthand != parent_sjinfo->syn_lefthand)
1774+
bms_free(child_sjinfo->syn_lefthand);
1775+
1776+
if (child_sjinfo->syn_righthand != parent_sjinfo->syn_righthand)
1777+
bms_free(child_sjinfo->syn_righthand);
1778+
1779+
Assert(child_sjinfo->commute_above_l == parent_sjinfo->commute_above_l);
1780+
Assert(child_sjinfo->commute_above_r == parent_sjinfo->commute_above_r);
1781+
Assert(child_sjinfo->commute_below_l == parent_sjinfo->commute_below_l);
1782+
Assert(child_sjinfo->commute_below_r == parent_sjinfo->commute_below_r);
1783+
1784+
Assert(child_sjinfo->semi_operators == parent_sjinfo->semi_operators);
17691785

17701786
/*
17711787
* semi_rhs_exprs may in principle be freed, but a simple pfree() does
17721788
* not suffice, so we leave it alone.
17731789
*/
17741790
}
17751791

1776-
pfree(sjinfo);
1792+
pfree(child_sjinfo);
17771793
}
17781794

17791795
/*

src/test/regress/expected/partition_join.out

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,41 @@ SELECT a, b FROM prt1 FULL JOIN prt2 p2(b,a,c) USING(a,b)
713713

714714
RESET enable_partitionwise_aggregate;
715715
RESET enable_hashjoin;
716+
-- bug in freeing the SpecialJoinInfo of a child-join
717+
EXPLAIN (COSTS OFF)
718+
SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3);
719+
QUERY PLAN
720+
--------------------------------------------------
721+
Append
722+
-> Hash Semi Join
723+
Hash Cond: (t1_1.a = t3_1.a)
724+
-> Hash Join
725+
Hash Cond: (t1_1.a = t2_1.a)
726+
-> Seq Scan on prt1_p1 t1_1
727+
-> Hash
728+
-> Seq Scan on prt1_p1 t2_1
729+
-> Hash
730+
-> Seq Scan on prt1_p1 t3_1
731+
-> Hash Semi Join
732+
Hash Cond: (t1_2.a = t3_2.a)
733+
-> Hash Join
734+
Hash Cond: (t1_2.a = t2_2.a)
735+
-> Seq Scan on prt1_p2 t1_2
736+
-> Hash
737+
-> Seq Scan on prt1_p2 t2_2
738+
-> Hash
739+
-> Seq Scan on prt1_p2 t3_2
740+
-> Hash Semi Join
741+
Hash Cond: (t1_3.a = t3_3.a)
742+
-> Hash Join
743+
Hash Cond: (t1_3.a = t2_3.a)
744+
-> Seq Scan on prt1_p3 t1_3
745+
-> Hash
746+
-> Seq Scan on prt1_p3 t2_3
747+
-> Hash
748+
-> Seq Scan on prt1_p3 t3_3
749+
(28 rows)
750+
716751
--
717752
-- partitioned by expression
718753
--

src/test/regress/sql/partition_join.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ SELECT a, b FROM prt1 FULL JOIN prt2 p2(b,a,c) USING(a,b)
143143
RESET enable_partitionwise_aggregate;
144144
RESET enable_hashjoin;
145145

146+
-- bug in freeing the SpecialJoinInfo of a child-join
147+
EXPLAIN (COSTS OFF)
148+
SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3);
149+
146150
--
147151
-- partitioned by expression
148152
--

0 commit comments

Comments
 (0)