Re: Considering fractional paths in Append node

From: Andrei Lepikhov <lepihov(at)gmail(dot)com>
To: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Cc: Nikita Malakhov <hukutoc(at)gmail(dot)com>, Andy Fan <zhihuifan1213(at)163(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: Considering fractional paths in Append node
Date: 2025-03-03 11:04:06
Message-ID: 85d66dfc-309d-4ae4-819e-ff8ccd73f9ff@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2/3/2025 09:53, Alexander Korotkov wrote:
> Hi, Andrei!
>
> On Fri, Dec 6, 2024 at 10:13 AM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>>
>> On 12/6/24 13:48, Andrei Lepikhov wrote:
>>> On 11/2/24 01:18, Nikita Malakhov wrote:
>>>> I've corrected failing test and created a patch at Commitfest:
>>>> https://commitfest.postgresql.org/51/5361/ <https://
>>>> commitfest.postgresql.org/51/5361/>
>>> I have played around with this feature, which looks promising for such a
>>> tiny change. It provides a 'bottom boundary' recommendation for
>>> appending subpaths, participating in the 'fractional branch' of paths.
>>> As I see it works consistently with the plans, created for plain tables
>>> filled with similar data.
>>> According to the proposal to change SeqScan logic, IMO, Andy is right.
>>> But it is a separate improvement because it wouldn't work in the case of
>>> LIMIT 10 or 100, as the newly added regression tests demonstrate.
>>> I think this feature gives sensible profit for partitionwise paths.
>>> Pushing this knowledge into subpaths could help postgres_fdw to reduce
>>> network traffic.
>>>
>> See the attached patch: regression tests added; *_ext function removed -
>> I think we wouldn't back-patch it into minor releases.
>
> Thank you for revising this patch. I've a couple of questions for you.
> 1. You revise get_cheapest_fractional_path() to reject parametrized
> paths in all the cases. Are you sure this wouldn't affect other
> callers of this function?
Yes, in my opinion, it may not affect any caller for now. As you can
see, the routine is used for upper-rels only. Such RelOptInfos can't
contain parameterised paths for now. I already attempted to change the
parameterisation code and let queries pull parameterisation from
subqueries and subplans, but it is too invasive. It seems this logic
will stay as it is for a long time.
I added a comment to explain this logic.
What's more, since all subpaths of an Append must have the same
parameterisation, we simplify the case and just don't discover this option.

> 2. As usage of root->tuple_fraction RelOptInfo it has been criticized,
> do you think we could limit this to some simple cases? For instance,
> check that RelOptInfo is the final result relation for given root.
I believe that using tuple_fraction is not an issue. Instead, it serves
as a flag that allows the upper-level optimisation to consider
additional options. The upper-level optimiser has more variants to
examine and will select the optimal path based on the knowledge
available at that level. Therefore, we're not introducing a mistake
here; we're simply adding extra work in the narrow case. However, having
only the bottom-up planning process, I don't see how we could avoid this
additional workload.

--
regards, Andrei Lepikhov

Attachment Content-Type Size
v2-0001-Teach-Append-to-consider-tuple_fraction-when-accu.patch text/plain 11.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2025-03-03 11:11:38 Re: Add an option to skip loading missing publication to avoid logical replication failure
Previous Message John Naylor 2025-03-03 10:58:43 Re: vacuumdb changes for stats import/export