On Mon, Sep 16, 2024 at 3:44 PM Andrei Lepikhov <lepi...@gmail.com> wrote:
> On 9/9/2024 12:36, Alexander Korotkov wrote:
> > Also, I agree it get it's wrong to directly copy RestrictInfo struct
> > in group_similar_or_args().  Instead, I've renamed
> > make_restrictinfo_internal() to make_plain_restrictinfo(), which is
> > intended to handle non-recursive cases when you've children already
> > wrapped with RestrictInfos.  make_plain_restrictinfo() now used in
> > group_similar_or_args().
> Great work. Thanks for doing this!
>
> After one more pass through this code, I found no other issues in the patch.
> Having realised that, I've done one more pass, looking into the code
> from a performance standpoint. It looks mostly ok, but In my opinion, in
> the cycle:
>
> foreach(lc, orclause->args)
> {
> }
>
> we should free the consts list before returning NULL on unsuccessful
> attempt. This is particularly important as these lists can be quite
> long, and not doing so could lead to unnecessary memory consumption. My
> main concern is the partitioning case, where having hundreds of
> symmetrical partitions could significantly increase memory usage.

Makes sense.  Please, check the attached patch freeing the consts list
while returning NULL from match_orclause_to_indexcol().

------
Regards,
Alexander Korotkov
Supabase

Attachment: v40-0002-Teach-bitmap-path-generation-about-transforming-.patch
Description: Binary data

Attachment: v40-0001-Transform-OR-clauses-to-SAOP-s-during-index-matc.patch
Description: Binary data

Reply via email to