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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f0da5b23038e3d7544fe00429215484969be06ff..8e8c7e395856872ca4221cd06e73f73224a63874 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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;
 

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 9ce8f43385ec83227bb633278d38d37ac91b0af8..1dc873ed255f85a6f51769c4e2c1ffc570861562 100644 (file)
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -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);
 

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 9941dfe65e4696c3fea3b88476de94f65e3b4d4f..b3ab169f78c0bbac8c83fc612dc2814f28adffb5 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2259,7 +2259,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;

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4ea300797fea792f8b86009cdf27248720620f2e..767d0ed7e437fe4ced660ddb60b2452165c99dda 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4431,7 +4431,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);
 
@@ -4465,7 +4465,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);
 

diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index f813b587f186aae8b77d83efcb0233123b216f42..783f3fe8f2d1c141a5e871c49aeaed4ae818f4b8 100644 (file)
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -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;

diff --git a/src/backend/utils/adt/jsonpath_gram.y b/src/backend/utils/adt/jsonpath_gram.y
index f87db8ccf6b4b3081f29cfffc990b2a5227708f8..9947fedfc196b5b1904345594f9f064e9d942b04 100644 (file)
--- a/src/backend/utils/adt/jsonpath_gram.y
+++ b/src/backend/utils/adt/jsonpath_gram.y
@@ -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);
 

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 076c3c019ff425852dfdcb9ce19cf8f0602ddf87..9db584169fce6c71e4837b3ba1dce50f55093e2f 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -8123,7 +8123,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)
                {
@@ -8132,12 +8132,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, ')');
@@ -8148,12 +8147,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, ')');

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 8f06e8212d74ea22a08736c3dae79a248b347c4f..b3825633c0ac16b1e709bbfdc870101054e60686 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -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);
 

diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 14ea2766adb30d1178ea9eadf2a47551e00f4a54..54447bb5eb63626b302d1e968467886a90ddab1a 100644 (file)
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -380,6 +380,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
@@ -396,7 +422,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)};
@@ -447,8 +473,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),