From dfd869a4774db6ddc5c3493508e83c20544488c3 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Wed, 27 Sep 2023 16:29:11 +0530
Subject: [PATCH 3/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

