Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 44b5d56

Browse files
committed
Tidy up code in get_cheapest_group_keys_order()
There are a few things that we could do a little better within get_cheapest_group_keys_order(): 1. We should be using list_free() rather than pfree() on a List. 2. We should use for_each_from() instead of manually coding a for loop to skip the first n elements of a List 3. list_truncate(list_copy(...), n) is not a great way to copy the first n elements of a list. Let's invent list_copy_head() for that. That way we don't need to copy the entire list just to truncate it directly afterwards. 4. We can simplify finding the cheapest cost by setting the cheapest cost variable to DBL_MAX. That allows us to skip special-casing the initial iteration of the loop. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvrGyL3ft8waEkncG9y5HDMu5TFFJB1paoTC8zi9YK97Nw@mail.gmail.com Backpatch-through: 15, where get_cheapest_group_keys_order was added.
1 parent 12c99c8 commit 44b5d56

File tree

3 files changed

+40
-16
lines changed

3 files changed

+40
-16
lines changed

src/backend/nodes/list.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,6 +1544,27 @@ list_copy(const List *oldlist)
15441544
return newlist;
15451545
}
15461546

1547+
/*
1548+
* Return a shallow copy of the specified list containing only the first 'len'
1549+
* elements. If oldlist is shorter than 'len' then we copy the entire list.
1550+
*/
1551+
List *
1552+
list_copy_head(const List *oldlist, int len)
1553+
{
1554+
List *newlist;
1555+
1556+
len = Min(oldlist->length, len);
1557+
1558+
if (len <= 0)
1559+
return NIL;
1560+
1561+
newlist = new_list(oldlist->type, len);
1562+
memcpy(newlist->elements, oldlist->elements, len * sizeof(ListCell));
1563+
1564+
check_list_invariants(newlist);
1565+
return newlist;
1566+
}
1567+
15471568
/*
15481569
* Return a shallow copy of the specified list, without the first N elements.
15491570
*/

src/backend/optimizer/path/pathkeys.c

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
*/
1818
#include "postgres.h"
1919

20+
#include <float.h>
21+
2022
#include "miscadmin.h"
2123
#include "access/stratnum.h"
2224
#include "catalog/pg_opfamily.h"
@@ -591,7 +593,7 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
591593

592594
ListCell *cell;
593595
PathkeyMutatorState mstate;
594-
double cheapest_sort_cost = -1.0;
596+
double cheapest_sort_cost = DBL_MAX;
595597

596598
int nFreeKeys;
597599
int nToPermute;
@@ -620,23 +622,23 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
620622
nToPermute = 4;
621623
if (nFreeKeys > nToPermute)
622624
{
623-
int i;
624625
PathkeySortCost *costs = palloc(sizeof(PathkeySortCost) * nFreeKeys);
626+
PathkeySortCost *cost = costs;
625627

626-
/* skip the pre-ordered pathkeys */
627-
cell = list_nth_cell(*group_pathkeys, n_preordered);
628-
629-
/* estimate cost for sorting individual pathkeys */
630-
for (i = 0; cell != NULL; i++, (cell = lnext(*group_pathkeys, cell)))
628+
/*
629+
* Estimate cost for sorting individual pathkeys skipping the
630+
* pre-ordered pathkeys.
631+
*/
632+
for_each_from(cell, *group_pathkeys, n_preordered)
631633
{
632-
List *to_cost = list_make1(lfirst(cell));
633-
634-
Assert(i < nFreeKeys);
634+
PathKey *pathkey = (PathKey *) lfirst(cell);
635+
List *to_cost = list_make1(pathkey);
635636

636-
costs[i].pathkey = lfirst(cell);
637-
costs[i].cost = cost_sort_estimate(root, to_cost, 0, nrows);
637+
cost->pathkey = pathkey;
638+
cost->cost = cost_sort_estimate(root, to_cost, 0, nrows);
639+
cost++;
638640

639-
pfree(to_cost);
641+
list_free(to_cost);
640642
}
641643

642644
/* sort the pathkeys by sort cost in ascending order */
@@ -646,9 +648,9 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
646648
* Rebuild the list of pathkeys - first the preordered ones, then the
647649
* rest ordered by cost.
648650
*/
649-
new_group_pathkeys = list_truncate(list_copy(*group_pathkeys), n_preordered);
651+
new_group_pathkeys = list_copy_head(*group_pathkeys, n_preordered);
650652

651-
for (i = 0; i < nFreeKeys; i++)
653+
for (int i = 0; i < nFreeKeys; i++)
652654
new_group_pathkeys = lappend(new_group_pathkeys, costs[i].pathkey);
653655

654656
pfree(costs);
@@ -689,7 +691,7 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
689691

690692
cost = cost_sort_estimate(root, var_group_pathkeys, n_preordered, nrows);
691693

692-
if (cost < cheapest_sort_cost || cheapest_sort_cost < 0)
694+
if (cost < cheapest_sort_cost)
693695
{
694696
list_free(new_group_pathkeys);
695697
new_group_pathkeys = list_copy(var_group_pathkeys);

src/include/nodes/pg_list.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,7 @@ extern void list_free(List *list);
599599
extern void list_free_deep(List *list);
600600

601601
extern pg_nodiscard List *list_copy(const List *list);
602+
extern pg_nodiscard List *list_copy_head(const List *oldlist, int len);
602603
extern pg_nodiscard List *list_copy_tail(const List *list, int nskip);
603604
extern pg_nodiscard List *list_copy_deep(const List *oldlist);
604605

0 commit comments

Comments
 (0)