On 2/3/2025 09:53, Alexander Korotkov wrote:
> Hi, Andrei!
>
> On Fri, Dec 6, 2024 at 10:13 AM Andrei Lepikhov <lepihov@gmail.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
