Re: MergeAppend could consider sorting cheapest child path

Hi, Alexander!

On Wed, May 7, 2025 at 12:06 PM Alexander Pyhalov
<a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
> Andrei Lepikhov писал(а) 2025-05-07 12:03:
> > On 7/5/2025 08:57, Alexander Pyhalov wrote:
> >> Andrei Lepikhov писал(а) 2025-05-07 08:02:
> >>> On 5/5/2025 15:56, Alexander Pyhalov wrote:
> >>>> Andrei Lepikhov писал(а) 2025-05-05 14:38:
> >>>> Also logic a bit differs if path is NULL. In
> >>>> get_cheapest_path_for_pathkeys_ext() we explicitly check for path
> >>>> being NULL, in get_cheapest_fractional_path_for_pathkeys_ext() only
> >>>> after calculating sort cost.
> >>>>
> >>>> I've tried to fix comments a bit and unified functions definitions.
> >>> Generally seems ok, I'm not a native speaker to judge the comments.
> >>> But:
> >>> if (base_path && path != base_path)
> >>>
> >>> What is the case in your mind where the base_path pointer still may
> >>> be null at that point?
> >>
> >> Hi.
> >>
> >> It seems if some childrel doesn't have valid pathlist, subpaths_valid
> >> would be false in add_paths_to_append_rel()
> >> and generate_orderedappend_paths() will not be called. So when we
> >> iterate over live_childrels,
> >> all of them will have cheapest_total path. This is why we can assert
> >> that base_path is not NULL.
> > I'm not sure I understand you correctly. Under which conditions will
> > rel->cheapest_total_path be set to NULL for a childrel? Could you
> > provide an example?
>
> Sorry, perhaps I was not clear enough. I've stated the opposite - it
> seems we can be sure that it's not NULL.

Thank you for your work on this subject!

I have the following question. I see patch changes some existing
plans from Sort(Append(...)) to MergeAppend(Sort(), ..., Sort(...)) or
even Materialize(MergeAppend(Sort(), ..., Sort(...))). This should be
some problem in cost_sort(). Otherwise, that would mean that Sort
node doesn't know how to do its job: explicit splitting dataset into
pieces then merging sorting result appears to be cheaper, but Sort
node contains merge-sort algorithm inside and it's supposed to be more
efficient. Could you, please, revise the patch to avoid these
unwanted changes?

------
Regards,
Alexander Korotkov
Supabase