Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning - Mailing list pgsql-hackers

From Amit Langote
Subject Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning
Date
Msg-id CA+HiwqGBtUWz6qbbZon6aDEVsC8B7Z9W0KY2=ZwaTt9He1Yg=w@mail.gmail.com
Whole thread Raw
In response to Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning  (Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>)
Responses Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning
List pgsql-hackers
On Wed, Sep 27, 2023 at 8:07 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> On Wed, Sep 27, 2023 at 2:30 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > +   /*
> > +    * But the list of operator OIDs and the list of expressions may be
> > +    * referenced somewhere else. Do not free those.
> > +    */
> >
> > I don't see build_child_join_sjinfo() copying any list of OIDs, so I'm
> > not sure what the comment is referring to.  Also, instead of "the list
> > of expressions", it might be more useful to mention semi_rhs_expr,
> > because that's the only one being copied.
>
> list of OID is semi_operators. It's copied as is from parent
> SpecialJoinInfo. But the way it's mentioned in the comment is
> confusing. I have modified the prologue of function to provide a
> general guidance on what can be freed and what should not be. and then
> specific examples. Let me know if this is more clear.

Thanks.  I would've kept the notes about specific members inside the
function.  Also, no need to have a note for pointers that are not
deep-copied to begin with, such as semi_operators.  IOW, something
like the following would have sufficed:

@@ -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)
@@ -1755,10 +1759,7 @@ free_child_sjinfo_members(SpecialJoinInfo *child_sjinfo)
    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. */
 }

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Annoying build warnings from latest Apple toolchain
Next
From: David Rowley
Date:
Subject: Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)