From c39392ebaeeff7dafbc255844bcc605aaef01d7d Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Wed, 19 Feb 2025 10:02:32 +0900 Subject: [PATCH] Fix freeing a child join's SpecialJoinInfo MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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: 孟令彬 Diagnosed-by: Tender Wang Author: Richard Guo Reviewed-by: Amit Langote Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/18806-d70b0c9fdf63dcbf@postgresql.org Backpatch-through: 17 --- src/backend/optimizer/path/joinrels.c | 34 ++++++++++++++----- src/test/regress/expected/partition_join.out | 35 ++++++++++++++++++++ src/test/regress/sql/partition_join.sql | 4 +++ 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index c2eb300ea9b..60d65762b5d 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -45,7 +45,8 @@ static void try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, static SpecialJoinInfo *build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo, Relids left_relids, Relids right_relids); -static void free_child_join_sjinfo(SpecialJoinInfo *sjinfo); +static void free_child_join_sjinfo(SpecialJoinInfo *child_sjinfo, + SpecialJoinInfo *parent_sjinfo); static void compute_partition_bounds(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, RelOptInfo *joinrel, SpecialJoinInfo *parent_sjinfo, @@ -1687,7 +1688,7 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, */ pfree(appinfos); bms_free(child_relids); - free_child_join_sjinfo(child_sjinfo); + free_child_join_sjinfo(child_sjinfo, parent_sjinfo); } } @@ -1754,18 +1755,33 @@ build_child_join_sjinfo(PlannerInfo *root, SpecialJoinInfo *parent_sjinfo, * SpecialJoinInfo are freed here. */ static void -free_child_join_sjinfo(SpecialJoinInfo *sjinfo) +free_child_join_sjinfo(SpecialJoinInfo *child_sjinfo, + SpecialJoinInfo *parent_sjinfo) { /* * Dummy SpecialJoinInfos of inner joins do not have any translated fields * and hence no fields that to be freed. */ - if (sjinfo->jointype != JOIN_INNER) + if (child_sjinfo->jointype != JOIN_INNER) { - bms_free(sjinfo->min_lefthand); - bms_free(sjinfo->min_righthand); - bms_free(sjinfo->syn_lefthand); - bms_free(sjinfo->syn_righthand); + if (child_sjinfo->min_lefthand != parent_sjinfo->min_lefthand) + bms_free(child_sjinfo->min_lefthand); + + if (child_sjinfo->min_righthand != parent_sjinfo->min_righthand) + bms_free(child_sjinfo->min_righthand); + + if (child_sjinfo->syn_lefthand != parent_sjinfo->syn_lefthand) + bms_free(child_sjinfo->syn_lefthand); + + if (child_sjinfo->syn_righthand != parent_sjinfo->syn_righthand) + bms_free(child_sjinfo->syn_righthand); + + Assert(child_sjinfo->commute_above_l == parent_sjinfo->commute_above_l); + Assert(child_sjinfo->commute_above_r == parent_sjinfo->commute_above_r); + Assert(child_sjinfo->commute_below_l == parent_sjinfo->commute_below_l); + Assert(child_sjinfo->commute_below_r == parent_sjinfo->commute_below_r); + + Assert(child_sjinfo->semi_operators == parent_sjinfo->semi_operators); /* * semi_rhs_exprs may in principle be freed, but a simple pfree() does @@ -1773,7 +1789,7 @@ free_child_join_sjinfo(SpecialJoinInfo *sjinfo) */ } - pfree(sjinfo); + pfree(child_sjinfo); } /* diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out index af468682a2d..34b963ce6fe 100644 --- a/src/test/regress/expected/partition_join.out +++ b/src/test/regress/expected/partition_join.out @@ -713,6 +713,41 @@ SELECT a, b FROM prt1 FULL JOIN prt2 p2(b,a,c) USING(a,b) RESET enable_partitionwise_aggregate; RESET enable_hashjoin; +-- bug in freeing the SpecialJoinInfo of a child-join +EXPLAIN (COSTS OFF) +SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3); + QUERY PLAN +-------------------------------------------------- + Append + -> Hash Semi Join + Hash Cond: (t1_1.a = t3_1.a) + -> Hash Join + Hash Cond: (t1_1.a = t2_1.a) + -> Seq Scan on prt1_p1 t1_1 + -> Hash + -> Seq Scan on prt1_p1 t2_1 + -> Hash + -> Seq Scan on prt1_p1 t3_1 + -> Hash Semi Join + Hash Cond: (t1_2.a = t3_2.a) + -> Hash Join + Hash Cond: (t1_2.a = t2_2.a) + -> Seq Scan on prt1_p2 t1_2 + -> Hash + -> Seq Scan on prt1_p2 t2_2 + -> Hash + -> Seq Scan on prt1_p2 t3_2 + -> Hash Semi Join + Hash Cond: (t1_3.a = t3_3.a) + -> Hash Join + Hash Cond: (t1_3.a = t2_3.a) + -> Seq Scan on prt1_p3 t1_3 + -> Hash + -> Seq Scan on prt1_p3 t2_3 + -> Hash + -> Seq Scan on prt1_p3 t3_3 +(28 rows) + -- -- partitioned by expression -- diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql index e84b65f4442..26b8e3d063f 100644 --- a/src/test/regress/sql/partition_join.sql +++ b/src/test/regress/sql/partition_join.sql @@ -143,6 +143,10 @@ SELECT a, b FROM prt1 FULL JOIN prt2 p2(b,a,c) USING(a,b) RESET enable_partitionwise_aggregate; RESET enable_hashjoin; +-- bug in freeing the SpecialJoinInfo of a child-join +EXPLAIN (COSTS OFF) +SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3); + -- -- partitioned by expression -- -- 2.30.2