Horton

Re: MergeAppend could consider sorting cheapest child path

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Cc: Andrei Lepikhov <lepihov@gmail.com>, Andy Fan <zhihuifan1213@163.com>, Bruce Momjian <bruce@momjian.us>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Nikita Malakhov <HukuToc@gmail.com>
Date: 2025-06-02T18:21:32Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Mostly-cosmetic adjustments to estimate_multivariate_bucketsize().

  2. Consider fractional paths in generate_orderedappend_paths

Hi, Alexander!

On Wed, May 7, 2025 at 12:06 PM Alexander Pyhalov
<a.pyhalov@postgrespro.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