Re: Properly pathify the union planner

On Fri, 24 Nov 2023 at 18:43, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> On Fri, Nov 24, 2023 at 3:59 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > For now, as a temporary fix, I've just #ifdef'd out the code in
> > add_path() that's pfreeing the old path. I have drafted a patch that
> > refcounts Paths, but I'm unsure if that's the correct solution as I'm
> > only maintaining the refcounts in add_path() and add_partial_path(). I
> > think a true correct solution would bump the refcount when a path is
> > used as some other path's subpath. That would mean having to
> > recursively pfree paths up until we find one with a refcount>0. Seems
> > a bit expensive for add_path() to do.
>
> Please find my proposal to refcount paths at [1]. I did that to reduce
> the memory consumed by partitionwise joins. I remember another thread
> where freeing a path that was referenced by upper sort path created
> minor debugging problem. [2]. I paused my work on my proposal since
> there didn't seem enough justification. But it looks like the
> requirement is coming up repeatedly. I am willing to resume my work if
> it's needed. The email lists next TODOs. As to making the add_path()
> expensive, I didn't find any noticeable impact on planning time.

I missed that thread. Thanks for pointing it out.

I skim read your patch and I see it does seem to have the workings for
tracking refcounts when the pack is a subpath of another path. I
imagine that would allow the undocumented hack that is "if
(!IsA(old_path, IndexPath))" in add_path() to disappear.

I wondered if the problem of pfreeing paths that are in the pathlist
of another relation could be fixed in another way. If we have an
AdoptedPath path type that just inherits the costs from its single
subpath and we wrap a Path up in one of these before we do add_path()
a Path which is not parented by the relation we're adding the path to,
since we don't recursively pfree() Paths in add_path(), we'd only ever
pfree the AdoptedPath rather than pfreeing a Path that directly exists
in another relations pathlist.

Another simpler option would be just don't pfree the Path if the Path
parent is not the add_path rel.

David

> [1] https://www.postgresql.org/message-id/CAExHW5tUcVsBkq9qT%3DL5vYz4e-cwQNw%3DKAGJrtSyzOp3F%3DXacA%40mail.gmail.com
> [2] https://www.postgresql.org/message-id/CAM2%2B6%3DUC1mcVtM0Y_LEMBEGHTM58HEkqHPn7vau_V_YfuZjEGg%40mail.gmail.com