8000 Tidy up code in get_cheapest_group_keys_order() · postgrespro/postgres@4cc832f · GitHub
[go: up one dir, main page]
Skip to content

Commit 4cc832f

Browse files
david-rowleydavid-rowley
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 83f1c7b commit 4cc832f

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
@@ -1584,6 +1584,27 @@ list_copy(const List *oldlist)
15841584
return newlist;
15851585
}
15861586

1587+
/*
1588+
* Return a shallow copy of the specified list containing only the first 'len'
1589+
* elements. If oldlist is shorter than 'len' then we copy the entire list.
1590+
*/
1591+
List *
1592+
list_copy_head(const List *oldlist, int len)
1593+
{
1594+
List *newlist;
1595+
1596+
len = Min(oldlist->length, len);
1597+
1598+
if (len <= 0)
1599+
return NIL;
1600+
1601+
newlist = new_list(oldlist->type, len);
1602+
memcpy(newlist->elements, oldlist->elements, len * sizeof(ListCell));
1603+
1604+
check_list_invariants(newlist);
1605+
return newlist;
1606+
}
1607+
15871608
/*
15881609
* Return a shallow copy of the specified list, without the first N elements.
15891610
*/

‎src/backend/optimizer/path/pathkeys.c

Lines changed: 18 additions & 16 deletions
< 10000 td data-grid-cell-id="diff-77ee840b90d875191508a796b3b780ec8cdc955c9b351301c5430f59234c56da-627-627-1" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-deletionNum-bgColor, var(--diffBlob-deletion-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">
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
@@ -620,6 +620,7 @@ extern void list_free(List *list);
620620
extern void list_free_deep(List *list);
621621

622622
extern pg_nodiscard List *list_copy(const List *list);
623+
extern pg_nodiscard List *list_copy_head(const List *oldlist, int len);
623624
extern pg_nodiscard List *lis 42E3 t_copy_tail(const List *list, int nskip);
624625
extern pg_nodiscard List *list_copy_deep(const List *oldlist);
625626

0 commit comments

Comments
 (0)
0