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

Commit 67b2cee

Browse files
committed
Add for_each_from, to simplify loops starting from non-first list cells.
We have a dozen or so places that need to iterate over all but the first cell of a List. Prior to v13 this was typically written as for_each_cell(lc, lnext(list_head(list))) Commit 1cff1b9 changed these to for_each_cell(lc, list, list_second_cell(list)) This patch introduces a new macro for_each_from() which expresses the start point as a list index, allowing these to be written as for_each_from(lc, list, 1) This is marginally more efficient, since ForEachState.i can be initialized directly instead of backing into it from a ListCell address. It also seems clearer and less typo-prone. Some of the remaining uses of for_each_cell() look like they could profitably be changed to for_each_from(), but here I confined myself to changing uses of list_second_cell(). Also, fix for_each_cell_setup() and for_both_cell_setup() to const-ify their arguments; that's a simple oversight in 1cff1b9. Back-patch into v13, on the grounds that (1) the const-ification is a minor bug fix, and (2) it's better for back-patching purposes if we only have two ways to write these loops rather than three. In HEAD, also remove list_third_cell() and list_fourth_cell(), which were also introduced in 1cff1b9, and are unused as of cc99baa43. It seems unlikely that any third-party code would have started to use them already; anyone who has can be directed to list_nth_cell instead. Discussion: https://postgr.es/m/CAApHDvpo1zj9KhEpU2cCRZfSM3Q6XGdhzuAS2v79PH7WJBkYVA@mail.gmail.com
1 parent 61a78c7 commit 67b2cee

File tree

9 files changed

+42
-18
lines changed

9 files changed

+42
-18
lines changed

src/backend/commands/tablecmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5732,7 +5732,7 @@ ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
57325732

57335733
inh = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
57345734
/* first element is the parent rel; must ignore it */
5735-
for_each_cell(cell, inh, list_second_cell(inh))
5735+
for_each_from(cell, inh, 1)
57365736
{
57375737
Relation childrel;
57385738

src/backend/nodes/nodeFuncs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ exprTypmod(const Node *expr)
441441
typmod = exprTypmod((Node *) linitial(cexpr->args));
442442
if (typmod < 0)
443443
return -1; /* no point in trying harder */
444-
for_each_cell(arg, cexpr->args, list_second_cell(cexpr->args))
444+
for_each_from(arg, cexpr->args, 1)
445445
{
446446
Node *e = (Node *) lfirst(arg);
447447

@@ -469,7 +469,7 @@ exprTypmod(const Node *expr)
469469
typmod = exprTypmod((Node *) linitial(mexpr->args));
470470
if (typmod < 0)
471471
return -1; /* no point in trying harder */
472-
for_each_cell(arg, mexpr->args, list_second_cell(mexpr->args))
472+
for_each_from(arg, mexpr->args, 1)
473473
{
474474
Node *e = (Node *) lfirst(arg);
475475

src/backend/optimizer/plan/createplan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2259,7 +2259,7 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
22592259
{
22602260
bool is_first_sort = ((RollupData *) linitial(rollups))->is_hashed;
22612261

2262-
for_each_cell(lc, rollups, list_second_cell(rollups))
2262+
for_each_from(lc, rollups, 1)
22632263
{
22642264
RollupData *rollup = lfirst(lc);
22652265
AttrNumber *new_grpColIdx;

src/backend/optimizer/plan/planner.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4431,7 +4431,7 @@ consider_groupingsets_paths(PlannerInfo *root,
44314431
* below, must use the same condition.
44324432
*/
44334433
i = 0;
4434-
for_each_cell(lc, gd->rollups, list_second_cell(gd->rollups))
4434+
for_each_from(lc, gd->rollups, 1)
44354435
{
44364436
RollupData *rollup = lfirst_node(RollupData, lc);
44374437

@@ -4465,7 +4465,7 @@ consider_groupingsets_paths(PlannerInfo *root,
44654465
rollups = list_make1(linitial(gd->rollups));
44664466

44674467
i = 0;
4468-
for_each_cell(lc, gd->rollups, list_second_cell(gd->rollups))
4468+
for_each_from(lc, gd->rollups, 1)
44694469
{
44704470
RollupData *rollup = lfirst_node(RollupData, lc);
44714471

src/backend/parser/parse_agg.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
10831083

10841084
if (gset_common)
10851085
{
1086-
for_each_cell(l, gsets, list_second_cell(gsets))
1086+
for_each_from(l, gsets, 1)
10871087
{
10881088
gset_common = list_intersection_int(gset_common, lfirst(l));
10891089
if (!gset_common)
@@ -1774,7 +1774,7 @@ expand_grouping_sets(List *groupingSets, int limit)
17741774
result = lappend(result, list_union_int(NIL, (List *) lfirst(lc)));
17751775
}
17761776

1777-
for_each_cell(lc, expanded_groups, list_second_cell(expanded_groups))
1777+
for_each_from(lc, expanded_groups, 1)
17781778
{
17791779
List *p = lfirst(lc);
17801780
List *new_result = NIL;

src/backend/utils/adt/jsonpath_gram.y

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ makeItemList(List *list)
441441
while (end->next)
442442
end = end->next;
443443

444-
for_each_cell(cell, list, list_second_cell(list))
444+
for_each_from(cell, list, 1)
445445
{
446446
JsonPathParseItem *c = (JsonPathParseItem *) lfirst(cell);
447447

src/backend/utils/adt/ruleutils.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8123,7 +8123,7 @@ get_rule_expr(Node *node, deparse_context *context,
81238123
{
81248124
BoolExpr *expr = (BoolExpr *) node;
81258125
Node *first_arg = linitial(expr->args);
8126-
ListCell *arg = list_second_cell(expr->args);
8126+
ListCell *arg;
81278127

81288128
switch (expr->boolop)
81298129
{
@@ -8132,12 +8132,11 @@ get_rule_expr(Node *node, deparse_context *context,
81328132
appendStringInfoChar(buf, '(');
81338133
get_rule_expr_paren(first_arg, context,
81348134
false, node);
8135-
while (arg)
8135+
for_each_from(arg, expr->args, 1)
81368136
{
81378137
appendStringInfoString(buf, " AND ");
81388138
get_rule_expr_paren((Node *) lfirst(arg), context,
81398139
false, node);
8140-
arg = lnext(expr->args, arg);
81418140
}
81428141
if (!PRETTY_PAREN(context))
81438142
appendStringInfoChar(buf, ')');
@@ -8148,12 +8147,11 @@ get_rule_expr(Node *node, deparse_context *context,
81488147
appendStringInfoChar(buf, '(');
81498148
get_rule_expr_paren(first_arg, context,
81508149
false, node);
8151-
while (arg)
8150+
for_each_from(arg, expr->args, 1)
81528151
{
81538152
appendStringInfoString(buf, " OR ");
81548153
get_rule_expr_paren((Node *) lfirst(arg), context,
81558154
false, node);
8156-
arg = lnext(expr->args, arg);
81578155
}
81588156
if (!PRETTY_PAREN(context))
81598157
appendStringInfoChar(buf, ')');

src/backend/utils/adt/selfuncs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3519,7 +3519,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
35193519
* for remaining Vars on other rels.
35203520
*/
35213521
relvarinfos = lappend(relvarinfos, varinfo1);
3522-
for_each_cell(l, varinfos, list_second_cell(varinfos))
3522+
for_each_from(l, varinfos, 1)
35233523
{
35243524
GroupVarInfo *varinfo2 = (GroupVarInfo *) lfirst(l);
35253525

src/include/nodes/pg_list.h

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,32 @@ lnext(const List *l, const ListCell *c)
380380
*/
381381
#define foreach_current_index(cell) (cell##__state.i)
382382

383+
/*
384+
* for_each_from -
385+
* Like foreach(), but start from the N'th (zero-based) list element,
386+
* not necessarily the first one.
387+
*
388+
* It's okay for N to exceed the list length, but not for it to be negative.
389+
*
390+
* The caveats for foreach() apply equally here.
391+
*/
392+
#define for_each_from(cell, lst, N) \
393+
for (ForEachState cell##__state = for_each_from_setup(lst, N); \
394+
(cell##__state.l != NIL && \
395+
cell##__state.i < cell##__state.l->length) ? \
396+
(cell = &cell##__state.l->elements[cell##__state.i], true) : \
397+
(cell = NULL, false); \
398+
cell##__state.i++)
399+
400+
static inline ForEachState
401+
for_each_from_setup(const List *lst, int N)
402+
{
403+
ForEachState r = {lst, N};
404+
405+
Assert(N >= 0);
406+
return r;
407+
}
408+
383409
/*
384410
* for_each_cell -
385411
* a convenience macro which loops through a list starting from a
@@ -396,7 +422,7 @@ lnext(const List *l, const ListCell *c)
396422
cell##__state.i++)
397423

398424
static inline ForEachState
399-
for_each_cell_setup(List *lst, ListCell *initcell)
425+
for_each_cell_setup(const List *lst, const ListCell *initcell)
400426
{
401427
ForEachState r = {lst,
402428
initcell ? list_cell_number(lst, initcell) : list_length(lst)};
@@ -447,8 +473,8 @@ for_each_cell_setup(List *lst, ListCell *initcell)
447473
cell1##__state.i1++, cell1##__state.i2++)
448474

449475
static inline ForBothCellState
450-
for_both_cell_setup(List *list1, ListCell *initcell1,
451-
List *list2, ListCell *initcell2)
476+
for_both_cell_setup(const List *list1, const ListCell *initcell1,
477+
const List *list2, const ListCell *initcell2)
452478
{
453479
ForBothCellState r = {list1, list2,
454480
initcell1 ? list_cell_number(list1, initcell1) : list_length(list1),

0 commit comments

Comments
 (0)