Redundant code in create_gather_merge_path

Started by Richard Guoabout 2 years ago3 messageshackers
Jump to latest
#1Richard Guo
guofenglinux@gmail.com

In create_gather_merge_path, we should always guarantee that the
subpath is adequately ordered, and we do not add a Sort node in
createplan.c for a Gather Merge node. Therefore, the 'else' branch in
the snippet from create_gather_merge_path is redundant.

if (pathkeys_contained_in(pathkeys, subpath->pathkeys))
{
/* Subpath is adequately ordered, we won't need to sort it */
input_startup_cost += subpath->startup_cost;
input_total_cost += subpath->total_cost;
}
else
{
/* We'll need to insert a Sort node, so include cost for that */
Path sort_path; /* dummy for result of cost_sort */

cost_sort(&sort_path,
root,
pathkeys,
subpath->total_cost,
subpath->rows,
subpath->pathtarget->width,
0.0,
work_mem,
-1);
input_startup_cost += sort_path.startup_cost;
input_total_cost += sort_path.total_cost;
}

We should be able to assert that pathkeys_contained_in(pathkeys,
subpath->pathkeys) is always true, otherwise we'll be in trouble.

I noticed this while reviewing patch [1]/messages/by-id/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com, thinking that it might be
worth fixing. Any thoughts?

[1]: /messages/by-id/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com

Thanks
Richard

#2Richard Guo
guofenglinux@gmail.com
In reply to: Richard Guo (#1)
Re: Redundant code in create_gather_merge_path

On Thu, Jul 18, 2024 at 10:02 AM Richard Guo <guofenglinux@gmail.com> wrote:

I noticed this while reviewing patch [1], thinking that it might be
worth fixing. Any thoughts?

Here is the patch.

Thanks
Richard

Attachments:

v1-0001-Remove-redundant-code-in-create_gather_merge_path.patchapplication/octet-stream; name=v1-0001-Remove-redundant-code-in-create_gather_merge_path.patchDownload+14-31
#3Richard Guo
guofenglinux@gmail.com
In reply to: Richard Guo (#2)
Re: Redundant code in create_gather_merge_path

On Thu, Jul 18, 2024 at 11:08 AM Richard Guo <guofenglinux@gmail.com> wrote:

On Thu, Jul 18, 2024 at 10:02 AM Richard Guo <guofenglinux@gmail.com> wrote:

I noticed this while reviewing patch [1], thinking that it might be
worth fixing. Any thoughts?

Here is the patch.

This patch is quite straightforward to remove the redundant code. So
I've gone ahead and pushed it.

Thanks
Richard