Re: Properly pathify the union planner

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Properly pathify the union planner
Date: 2023-11-24 05:43:04
Message-ID: CAExHW5tMnuTnKb8C668argOD4ig-Sb4=nrB8PkCjjd7DQcUCNQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 24, 2023 at 3:59 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> Another problem I hit was add_path() pfreeing a Path that I needed.
> This was happening due to how I'm building the final paths in the
> subquery when setop_pathkeys are set. Because I want to always
> include the cheapest_input_path to allow that path to be used in
> hash-based UNIONs, I also want to provide sorted paths so that
> MergeAppend has something to work with. I found cases where I'd
> add_path() the cheapest_input_path to the final rel then also attempt
> to sort that path. Unfortunately, add_path() found the unsorted path
> and the sorted path fuzzily the same cost and opted to keep the sorted
> one due to it having better pathkeys. add_path() then pfree'd the
> cheapest_input_path which meant the Sort's subpath was gone which
> obviously caused issues in createplan.c.
>
> 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.

[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

--
Best Wishes,
Ashutosh Bapat

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-11-24 05:45:06 Re: [PATCH] fix race condition in libpq (related to ssl connections)
Previous Message Alena Rybakina 2023-11-24 05:05:14 Re: POC, WIP: OR-clause support for indexes