On Fri, 1 Dec 2023 at 19:59, Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > I am fine to work on this further if the community thinks it is > acceptable. It seems from your point of view the worth of this work is > as follows > a. memory savings - not worth it > b. getting rid of !IsA(old_path, IndexPath) - worth it > c. problem of same path being added to multiple relations - not worth > it since you have other solution > > Can't judge whether that means it's acceptable or not.
I think it's worth trying to get this working and we can run some performance tests to see if it's cheap enough to use. I've not had enough time to fully process your patches, but on a quick look, I don't quite understand why link_path() does not have to recursively increment ref_counts in each of its subpaths. It seems you're doing the opposite in free_path(), so it seems like this would break for cases like a SortPath on say, a LimitPath over a SeqScan. When you call create_sort_path you'll only increment the refcount on the LimitPath. The SeqScan path then has a refcount 1 lower than it should. If something calls unlink_path on the SeqScan path that could put the refcount to 0 and break the SortPath by freeing a Path indirectly referenced by the sort. Recursively incrementing the refcounts would slow the patch down a bit, so we'd need to test the performance of this. I think at the rate we create and free join paths in complex join problems we could end up bumping refcounts quite often. Another way to fix the pfree issues was to add infrastructure to shallow copy paths. We could shallow copy paths whenever we borrow a Path from another RelOptInfo and then just reset the parent to the new RelOptInfo. That unfortunately makes your memory issue a bit worse. We discussed this a bit in [1]. It also seems there was some discussion about wrapping a Path up in a ProjectionPath in there. That unfortunately also takes us in the opposite direction in terms of your memory reduction goals. Maybe we can try to move forward with your refcount idea and see how the performance looks. If that's intolerable then that might help us decide on the next best alternative solution. David [1] https://postgr.es/m/caaphdvo8tib5xbja94f4mo8of2fpj2zg+zqqqtgr-thjssy...@mail.gmail.com