On 18/7/2025 13:48, Ashutosh Bapat wrote:
> On Mon, Jul 7, 2025 at 8:43 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
> if (!IsA(new_path, IndexPath))
> - pfree(new_path);
> + free_path(new_path, 0, false);
>
> Why don't we free the subpaths if they aren't referenced anymore?
During testing, I discovered that we sometimes encounter unusual cases.
For example, imagine pathlist:
{SeqScan, IndexOnlyScan1, IndexScan2}
Someone may decide that Sort+SeqScan may be cheaper than IndexScan2. Or,
IncrementalSort+IndexOnlyScan1 is cheaper than IndexScan2 ... And add
this path to the same pathlist. I am unsure which exact combinations may
arise in the planner, but without strict rules, someone may forget to
increment the refcounter.
>> To address this complexity, I propose the following solutions:
>> 1. Localise reference management within the add_path function.
>> 2. Transition from a 'strict' approach, which removes all unused paths,
>> to a more straightforward 'pinning' method. In this approach, we would
>> simply mark a path as 'used' when someone who was added to the upper
>> path list references it. Removing less memory, we leave the code much
>> simpler.
> Yes. This was one of the ideas Tom had proposed earlier in another
> thread to manage paths better and avoid dangling pointers. May be it's
> worth starting with that first. Get rid of special handling of index
> paths and then improve the memory situation. However, even with that,
> I think we should free more paths than less.
It seems like one more trade-off: more eager cleaning means more
resources spent.
P.S. path_walker makes sense to implement.
--
regards, Andrei Lepikhov
