Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning

From: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Richard Guo <guofenglinux(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Memory consumed by child SpecialJoinInfo in partitionwise join planning
Date: 2024-02-14 07:33:39
Message-ID: bfdfc349-3d33-45e7-ab4d-392f2d606e85@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 14/2/2024 13:32, Ashutosh Bapat wrote:
> On Wed, Feb 14, 2024 at 9:50 AM Andrei Lepikhov
> <a(dot)lepikhov(at)postgrespro(dot)ru> wrote:
>>
>> On 30/1/2024 12:44, Ashutosh Bapat wrote:
>>> Thanks Vignesh. PFA patches rebased on the latest HEAD. The patch
>>> addressing Amit's comments is still a separate patch for him to
>>> review.
>> Thanks for this improvement. Working with partitions, I frequently see
>> peaks of memory consumption during planning. So, maybe one more case can
>> be resolved here.
>> Patch 0001 looks good. I'm not sure about free_child_sjinfo_members. Do
>> we really need it as a separate routine? It might be better to inline
>> this code.
>
> try_partitionwise_join() is already 200 lines long. A separate
> function is better than adding more lines to try_partitionwise_join().
> Also if someone wants to call build_child_join_sjinfo() outside
> try_partitionwise_join() may find free_child_sjinfo_members() handy.
Make sense, thanks.
>> Patch 0002 adds valuable comments, and I'm OK with that.
>>
>> Also, as I remember, some extensions, such as pg_hint_plan, call
>> build_child_join_sjinfo. It is OK to break the interface with a major
>> version. But what if they need child_sjinfo a bit longer and collect
>> links to this structure? I don't think it is a real stopper, but it is
>> worth additional analysis.
>
> If these extensions call build_child_join_sjinfo() and do not call
> free_child_sjinfo_members, they can keep their child sjinfo as long as
> they want. I didn't understand your concern.
Nothing special. This patch makes external code responsible for
allocation of the structure and it adds more paths to do something
wrong. Current code looks good.

--
regards,
Andrei Lepikhov
Postgres Professional

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2024-02-14 07:34:37 Re: About a recently-added message
Previous Message Kyotaro Horiguchi 2024-02-14 07:26:52 About a recently-added message