0003-Use-palloc-ed-SpecialJoinInfo-instead-of-on-20240318.patch

text/x-patch

Filename: 0003-Use-palloc-ed-SpecialJoinInfo-instead-of-on-20240318.patch
Type: text/x-patch
Part: 2
Message: Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

Patch

Format: format-patch
Series: patch 0003
Subject: Use palloc'ed SpecialJoinInfo instead of one on the stack
File+
src/backend/optimizer/path/joinrels.c 32 24
From 26eea1ff25ac419c38f8a6d157acb37ebcf539b7 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@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