Skip to content

Commit 56fe008

Browse files
tglsfdctglsfdc
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 cc99baa. 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 fe0a1dc commit 56fe008

File tree

9 files changed

+42
-38
lines changed

9 files changed

+42
-38
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
@@ -2261,7 +2261,7 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
22612261
{
22622262
bool is_first_sort = ((RollupData *) linitial(rollups))->is_hashed;
22632263

2264-
for_each_cell(lc, rollups, list_second_cell(rollups))
2264+
for_each_from(lc, rollups, 1)
22652265
{
22662266
RollupData *rollup = lfirst(lc);
22672267
AttrNumber *new_grpColIdx;

‎src/backend/optimizer/plan/planner.c

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

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

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

‎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
@@ -8113,7 +8113,7 @@ get_rule_expr(Node *node, deparse_context *context,
81138113
{
81148114
BoolExpr *expr = (BoolExpr *) node;
81158115
Node *first_arg = linitial(expr->args);
8116-
ListCell *arg = list_second_cell(expr->args);
8116+
ListCell *arg;
81178117

81188118
switch (expr->boolop)
81198119
{
@@ -8122,12 +8122,11 @@ get_rule_expr(Node *node, deparse_context *context,
81228122
appendStringInfoChar(buf, '(');
81238123
get_rule_expr_paren(first_arg, context,
81248124
false, node);
8125-
while (arg)
8125+
for_each_from(arg, expr->args, 1)
81268126
{
81278127
appendStringInfoString(buf, " AND ");
81288128
get_rule_expr_paren((Node *) lfirst(arg), context,
81298129
false, node);
8130-
arg = lnext(expr->args, arg);
81318130
}
81328131
if (!PRETTY_PAREN(context))
81338132
appendStringInfoChar(buf, ')');
@@ -8138,12 +8137,11 @@ get_rule_expr(Node *node, deparse_context *context,
81388137
appendStringInfoChar(buf, '(');
81398138
get_rule_expr_paren(first_arg, context,
81408139
false, node);
8141-
while (arg)
8140+
for_each_from(arg, expr->args, 1)
81428141
{
81438142
appendStringInfoString(buf, " OR ");
81448143
get_rule_expr_paren((Node *) lfirst(arg), context,
81458144
false, node);
8146-
arg = lnext(expr->args, arg);
81478145
}
81488146
if (!PRETTY_PAREN(context))
81498147
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 & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -144,26 +144,6 @@ list_second_cell(const List *l)
144144
return NULL;
145145
}
146146

147-
/* Fetch address of list's third cell, if it has one, else NULL */
148-
static inline ListCell *
149-
list_third_cell(const List *l)
150-
{
151-
if (l && l->length >= 3)
152-
return &l->elements[2];
153-
else
154-
return NULL;
155-
}
156-
157-
/* Fetch address of list's fourth cell, if it has one, else NULL */
158-
static inline ListCell *
159-
list_fourth_cell(const List *l)
160-
{
161-
if (l && l->length >= 4)
162-
return &l->elements[3];
163-
else
164-
return NULL;
165-
}
166-
167147
/* Fetch list's length */
168148
static inline int
169149
list_length(const List *l)
@@ -389,6 +369,32 @@ lnext(const List *l, const ListCell *c)
389369
*/
390370
#define foreach_current_index(cell) (cell##__state.i)
391371

372+
/*
373+
* for_each_from -
374+
* Like foreach(), but start from the N'th (zero-based) list element,
375+
* not necessarily the first one.
376+
*
377+
* It's okay for N to exceed the list length, but not for it to be negative.
378+
*
379+
* The caveats for foreach() apply equally here.
380+
*/
381+
#define for_each_from(cell, lst, N) \
382+
for (ForEachState cell##__state = for_each_from_setup(lst, N); \
383+
(cell##__state.l != NIL && \
384+
cell##__state.i < cell##__state.l->length) ? \
385+
(cell = &cell##__state.l->elements[cell##__state.i], true) : \
386+
(cell = NULL, false); \
387+
cell##__state.i++)
388+
389+
static inline ForEachState
390+
for_each_from_setup(const List *lst, int N)
391+
{
392+
ForEachState r = {lst, N};
393+
394+
Assert(N >= 0);
395+
return r;
396+
}
397+
392398
/*
393399
* for_each_cell -
394400
* a convenience macro which loops through a list starting from a
@@ -405,7 +411,7 @@ lnext(const List *l, const ListCell *c)
405411
cell##__state.i++)
406412

407413
static inline ForEachState
408-
for_each_cell_setup(List *lst, ListCell *initcell)
414+
for_each_cell_setup(const List *lst, const ListCell *initcell)
409415
{
410416
ForEachState r = {lst,
411417
initcell ? list_cell_number(lst, initcell) : list_length(lst)};
@@ -456,8 +462,8 @@ for_each_cell_setup(List *lst, ListCell *initcell)
456462
cell1##__state.i1++, cell1##__state.i2++)
457463

458464
static inline ForBothCellState
459-
for_both_cell_setup(List *list1, ListCell *initcell1,
460-
List *list2, ListCell *initcell2)
465+
for_both_cell_setup(const List *list1, const ListCell *initcell1,
466+
const List *list2, const ListCell *initcell2)
461467
{
462468
ForBothCellState r = {list1, list2,
463469
initcell1 ? list_cell_number(list1, initcell1) : list_length(list1),

0 commit comments

Comments
 (0)