Re: Considering fractional paths in Append node

On Mon, Mar 3, 2025 at 1:04 PM Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>
> 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.

Makes sense. I have tries to add Assert(!path->param_info) to
get_cheapest_fractional_path() without the patch, and tests get
passed.

> > 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.

Yes, but if we can assume root->tuple_fraction applies to result of
Append, it's strange we apply the same tuple fraction to all the child
rels. Latter rels should less likely be used at all and perhaps
should have less tuple_fraction.

------
Regards,
Alexander Korotkov
Supabase