On 18/7/2025 13:48, Ashutosh Bapat wrote:
On Mon, Jul 7, 2025 at 8:43 PM Andrei Lepikhov <lepi...@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
