Redundant code in create_gather_merge_path
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
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
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