Re: Memory consumed by paths during partitionwise join planning

On Fri, 8 Dec 2023 at 18:02, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
> given path. E.g. we have three path chains as follows
> 1. joinpath_1->joinpath_2->seqpath_1,
> 2. joinpath_3->joinpath_4->seqpath_1,
> 3. joinpath_5->joinpath_2->seqpath_1
>
> Please note that this is not full path tree/forest. It is difficult to
> describe the whole path forest in text format. But this should be
> sufficient to get the gist of how linking and unlinking works.
>
> Let's say all these paths are listed in pathlists of respective
> RelOptInfos. Assume that joinpath_1, joinpath_3 and joinpath_5 are all
> part of the topmost RelOptInfo. Then the refcounts will be as follows
> joinpath_1, joinpath_3 and joinpath_5 -> each 1 (RelOptInfo)
> joinpath_2 - 3 (joinpath_2, joinpath_5 and RelOptInfo)
> joinpath_4 - 2 (joinpath_3, RelOptInfo)
> seqpath_1 - 3 (joinpath_2, joinpath_4, RelOptInfo)

I think this likely works fine providing you always unlink paths from
the root of the path tree.  When you start unlinking from non-root
paths I think you could start to see problems unless you increment
refcounts recursively.

The problem I had when working on improving the union planner was when
building the final paths for a union child.

Let's say I have the query:  SELECT a,b FROM t1 UNION SELECT x,y FROM t2;

When planning the first union child, I'd like to provide the union
planner with a sorted path and also the cheapest scan path on t1 to
allow it to Hash Aggregate on the cheapest path.

Let's say the planner produces the following paths on t1.

SeqScan_t1

When I create the final union child paths I want to try and produce a
few sorted paths to allow MergeAppend to work. I'll loop over each
path in t1's pathlist.

SeqScan_t1 isn't sorted, so I'll make Sort1->SeqScan_t1 add add_path
that to final_rel.

Now, I also want to allow cheap Hash Aggregates to implement the
UNION, so I'll include SeqScan_t1 without sorting it.

If I now do add_path(final_rel, SeqScan_t1), add_path() could see that
the SeqScan cost is fuzzily the same as the Sort->SeqScan_t1 and since
the sort version has better PathKeys, add_path will unlink SeqScan_t1.

Now, I don't think your scheme for refcounting paths is broken by this
because you'll increment the refcount of the SeqScan_t1 when the Sort
uses it as its subpath, but if instead of a Sort->SeqScan that was
IncrementalSort->Sort->SeqScan, then suddenly you're not incrementing
the refcount for the SeqScan when you create the Incremental Sort due
to the Sort being between the two.

So, if I now do add_path(final_rel, Sort2->Incremental_Sort->SeqScan_t1)

Sort1 is rejected due to cost and we unlink it. Sort1 refcount gets to
0 and we free the path and the SeqScan_t1's refcount goes down by 1
but does not reach 0.

We then add_path(final_rel, SeqScan_t1) but this path is fuzzily the
same cost as Sort2 so we reject SeqScan_t1 due to Sort2 having better
pathkeys and we unlink SeqScan_t1.  Refcount on SeqScan_t1 gets to 0
and we free the path. We need SeqScan_t1 for the incremental sort.

Perhaps in reality here, since SeqScan_t1 exists in the base rel's
pathlist we'd still have a refcount from that, but I assume at some
point you want to start freeing up paths from RelOptInfos that we're
done with, in which case the SeqScan_t1's refcount would reach 0 at
that point and we'd crash during create plan of the
Sort2->Incremental_Sort->SeqScan_t1 plan.

Have I misunderstood something here?  As far as I see it with your
non-recursive refcounts, it's only safe to free from the root of a
pathtree and isn't safe when unlinking paths that are the subpath of
another path unless the unlinking is done from the root.

David