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 1cff1b95a 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 1cff1b95a.

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 1cff1b95a, 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
This commit is contained in:
Tom Lane 2020-09-28 20:32:53 -04:00
parent fe0a1dc52c
commit 56fe008996
9 changed files with 42 additions and 38 deletions

View File

@ -5732,7 +5732,7 @@ ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
inh = find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
/* first element is the parent rel; must ignore it */
for_each_cell(cell, inh, list_second_cell(inh))
for_each_from(cell, inh, 1)
{
Relation childrel;

View File

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

View File

@ -2261,7 +2261,7 @@ create_groupingsets_plan(PlannerInfo *root, GroupingSetsPath *best_path)
{
bool is_first_sort = ((RollupData *) linitial(rollups))->is_hashed;
for_each_cell(lc, rollups, list_second_cell(rollups))
for_each_from(lc, rollups, 1)
{
RollupData *rollup = lfirst(lc);
AttrNumber *new_grpColIdx;

View File

@ -4430,7 +4430,7 @@ consider_groupingsets_paths(PlannerInfo *root,
* below, must use the same condition.
*/
i = 0;
for_each_cell(lc, gd->rollups, list_second_cell(gd->rollups))
for_each_from(lc, gd->rollups, 1)
{
RollupData *rollup = lfirst_node(RollupData, lc);
@ -4464,7 +4464,7 @@ consider_groupingsets_paths(PlannerInfo *root,
rollups = list_make1(linitial(gd->rollups));
i = 0;
for_each_cell(lc, gd->rollups, list_second_cell(gd->rollups))
for_each_from(lc, gd->rollups, 1)
{
RollupData *rollup = lfirst_node(RollupData, lc);

View File

@ -1083,7 +1083,7 @@ parseCheckAggregates(ParseState *pstate, Query *qry)
if (gset_common)
{
for_each_cell(l, gsets, list_second_cell(gsets))
for_each_from(l, gsets, 1)
{
gset_common = list_intersection_int(gset_common, lfirst(l));
if (!gset_common)
@ -1774,7 +1774,7 @@ expand_grouping_sets(List *groupingSets, int limit)
result = lappend(result, list_union_int(NIL, (List *) lfirst(lc)));
}
for_each_cell(lc, expanded_groups, list_second_cell(expanded_groups))
for_each_from(lc, expanded_groups, 1)
{
List *p = lfirst(lc);
List *new_result = NIL;

View File

@ -441,7 +441,7 @@ makeItemList(List *list)
while (end->next)
end = end->next;
for_each_cell(cell, list, list_second_cell(list))
for_each_from(cell, list, 1)
{
JsonPathParseItem *c = (JsonPathParseItem *) lfirst(cell);

View File

@ -8113,7 +8113,7 @@ get_rule_expr(Node *node, deparse_context *context,
{
BoolExpr *expr = (BoolExpr *) node;
Node *first_arg = linitial(expr->args);
ListCell *arg = list_second_cell(expr->args);
ListCell *arg;
switch (expr->boolop)
{
@ -8122,12 +8122,11 @@ get_rule_expr(Node *node, deparse_context *context,
appendStringInfoChar(buf, '(');
get_rule_expr_paren(first_arg, context,
false, node);
while (arg)
for_each_from(arg, expr->args, 1)
{
appendStringInfoString(buf, " AND ");
get_rule_expr_paren((Node *) lfirst(arg), context,
false, node);
arg = lnext(expr->args, arg);
}
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');
@ -8138,12 +8137,11 @@ get_rule_expr(Node *node, deparse_context *context,
appendStringInfoChar(buf, '(');
get_rule_expr_paren(first_arg, context,
false, node);
while (arg)
for_each_from(arg, expr->args, 1)
{
appendStringInfoString(buf, " OR ");
get_rule_expr_paren((Node *) lfirst(arg), context,
false, node);
arg = lnext(expr->args, arg);
}
if (!PRETTY_PAREN(context))
appendStringInfoChar(buf, ')');

View File

@ -3519,7 +3519,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
* for remaining Vars on other rels.
*/
relvarinfos = lappend(relvarinfos, varinfo1);
for_each_cell(l, varinfos, list_second_cell(varinfos))
for_each_from(l, varinfos, 1)
{
GroupVarInfo *varinfo2 = (GroupVarInfo *) lfirst(l);

View File

@ -144,26 +144,6 @@ list_second_cell(const List *l)
return NULL;
}
/* Fetch address of list's third cell, if it has one, else NULL */
static inline ListCell *
list_third_cell(const List *l)
{
if (l && l->length >= 3)
return &l->elements[2];
else
return NULL;
}
/* Fetch address of list's fourth cell, if it has one, else NULL */
static inline ListCell *
list_fourth_cell(const List *l)
{
if (l && l->length >= 4)
return &l->elements[3];
else
return NULL;
}
/* Fetch list's length */
static inline int
list_length(const List *l)
@ -389,6 +369,32 @@ lnext(const List *l, const ListCell *c)
*/
#define foreach_current_index(cell) (cell##__state.i)
/*
* for_each_from -
* Like foreach(), but start from the N'th (zero-based) list element,
* not necessarily the first one.
*
* It's okay for N to exceed the list length, but not for it to be negative.
*
* The caveats for foreach() apply equally here.
*/
#define for_each_from(cell, lst, N) \
for (ForEachState cell##__state = for_each_from_setup(lst, N); \
(cell##__state.l != NIL && \
cell##__state.i < cell##__state.l->length) ? \
(cell = &cell##__state.l->elements[cell##__state.i], true) : \
(cell = NULL, false); \
cell##__state.i++)
static inline ForEachState
for_each_from_setup(const List *lst, int N)
{
ForEachState r = {lst, N};
Assert(N >= 0);
return r;
}
/*
* for_each_cell -
* a convenience macro which loops through a list starting from a
@ -405,7 +411,7 @@ lnext(const List *l, const ListCell *c)
cell##__state.i++)
static inline ForEachState
for_each_cell_setup(List *lst, ListCell *initcell)
for_each_cell_setup(const List *lst, const ListCell *initcell)
{
ForEachState r = {lst,
initcell ? list_cell_number(lst, initcell) : list_length(lst)};
@ -456,8 +462,8 @@ for_each_cell_setup(List *lst, ListCell *initcell)
cell1##__state.i1++, cell1##__state.i2++)
static inline ForBothCellState
for_both_cell_setup(List *list1, ListCell *initcell1,
List *list2, ListCell *initcell2)
for_both_cell_setup(const List *list1, const ListCell *initcell1,
const List *list2, const ListCell *initcell2)
{
ForBothCellState r = {list1, list2,
initcell1 ? list_cell_number(list1, initcell1) : list_length(list1),