From c94fb8e8acc05c4b5f9f5b2a595ce7930827c2be Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 28 Feb 2019 14:25:01 -0500 Subject: [PATCH] Standardize some more loops that chase down parallel lists. We have forboth() and forthree() macros that simplify iterating through several parallel lists, but not everyplace that could reasonably use those was doing so. Also invent forfour() and forfive() macros to do the same for four or five parallel lists, and use those where applicable. The immediate motivation for doing this is to reduce the number of ad-hoc lnext() calls, to reduce the footprint of a WIP patch. However, it seems like good cleanup and error-proofing anyway; the places that were combining forthree() with a manually iterated loop seem particularly illegible and bug-prone. There was some speculation about restructuring related parsetree representations to reduce the need for parallel list chasing of this sort. Perhaps that's a win, or perhaps not, but in any case it would be considerably more invasive than this patch; and it's not particularly related to my immediate goal of improving the List infrastructure. So I'll leave that question for another day. Patch by me; thanks to David Rowley for review. Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us --- src/backend/access/common/tupdesc.c | 19 +++--------- src/backend/executor/execExpr.c | 20 +++--------- src/backend/executor/nodeIndexscan.c | 28 +++++++---------- src/backend/optimizer/plan/subselect.c | 5 +-- src/backend/optimizer/prep/prepunion.c | 7 ++--- src/backend/parser/analyze.c | 42 ++++++++------------------ src/backend/parser/parse_func.c | 7 ++--- src/backend/utils/adt/ruleutils.c | 37 +++++++---------------- src/include/nodes/pg_list.h | 26 ++++++++++++++++ 9 files changed, 76 insertions(+), 115 deletions(-) diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index 47e80ae186..832c3e9af6 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -902,23 +902,12 @@ BuildDescFromLists(List *names, List *types, List *typmods, List *collations) desc = CreateTemplateTupleDesc(natts); attnum = 0; - - l2 = list_head(types); - l3 = list_head(typmods); - l4 = list_head(collations); - foreach(l1, names) + forfour(l1, names, l2, types, l3, typmods, l4, collations) { char *attname = strVal(lfirst(l1)); - Oid atttypid; - int32 atttypmod; - Oid attcollation; - - atttypid = lfirst_oid(l2); - l2 = lnext(l2); - atttypmod = lfirst_int(l3); - l3 = lnext(l3); - attcollation = lfirst_oid(l4); - l4 = lnext(l4); + Oid atttypid = lfirst_oid(l2); + int32 atttypmod = lfirst_int(l3); + Oid attcollation = lfirst_oid(l4); attnum++; diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index db3777d15e..7cbf9d3bc1 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1683,7 +1683,6 @@ ExecInitExprRec(Expr *node, ExprState *state, *l_opfamily, *l_inputcollid; ListCell *lc; - int off; /* * Iterate over each field, prepare comparisons. To handle @@ -1695,20 +1694,11 @@ ExecInitExprRec(Expr *node, ExprState *state, Assert(list_length(rcexpr->opfamilies) == nopers); Assert(list_length(rcexpr->inputcollids) == nopers); - off = 0; - for (off = 0, - l_left_expr = list_head(rcexpr->largs), - l_right_expr = list_head(rcexpr->rargs), - l_opno = list_head(rcexpr->opnos), - l_opfamily = list_head(rcexpr->opfamilies), - l_inputcollid = list_head(rcexpr->inputcollids); - off < nopers; - off++, - l_left_expr = lnext(l_left_expr), - l_right_expr = lnext(l_right_expr), - l_opno = lnext(l_opno), - l_opfamily = lnext(l_opfamily), - l_inputcollid = lnext(l_inputcollid)) + forfive(l_left_expr, rcexpr->largs, + l_right_expr, rcexpr->rargs, + l_opno, rcexpr->opnos, + l_opfamily, rcexpr->opfamilies, + l_inputcollid, rcexpr->inputcollids) { Expr *left_expr = (Expr *) lfirst(l_left_expr); Expr *right_expr = (Expr *) lfirst(l_right_expr); diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 324356ec75..8b29437893 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -1332,12 +1332,12 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, { /* (indexkey, indexkey, ...) op (expression, expression, ...) */ RowCompareExpr *rc = (RowCompareExpr *) clause; - ListCell *largs_cell = list_head(rc->largs); - ListCell *rargs_cell = list_head(rc->rargs); - ListCell *opnos_cell = list_head(rc->opnos); - ListCell *collids_cell = list_head(rc->inputcollids); ScanKey first_sub_key; int n_sub_key; + ListCell *largs_cell; + ListCell *rargs_cell; + ListCell *opnos_cell; + ListCell *collids_cell; Assert(!isorderby); @@ -1346,19 +1346,22 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, n_sub_key = 0; /* Scan RowCompare columns and generate subsidiary ScanKey items */ - while (opnos_cell != NULL) + forfour(largs_cell, rc->largs, rargs_cell, rc->rargs, + opnos_cell, rc->opnos, collids_cell, rc->inputcollids) { ScanKey this_sub_key = &first_sub_key[n_sub_key]; int flags = SK_ROW_MEMBER; Datum scanvalue; Oid inputcollation; + leftop = (Expr *) lfirst(largs_cell); + rightop = (Expr *) lfirst(rargs_cell); + opno = lfirst_oid(opnos_cell); + inputcollation = lfirst_oid(collids_cell); + /* * leftop should be the index key Var, possibly relabeled */ - leftop = (Expr *) lfirst(largs_cell); - largs_cell = lnext(largs_cell); - if (leftop && IsA(leftop, RelabelType)) leftop = ((RelabelType *) leftop)->arg; @@ -1374,9 +1377,6 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, * We have to look up the operator's associated btree support * function */ - opno = lfirst_oid(opnos_cell); - opnos_cell = lnext(opnos_cell); - if (index->rd_rel->relam != BTREE_AM_OID || varattno < 1 || varattno > indnkeyatts) elog(ERROR, "bogus RowCompare index qualification"); @@ -1398,15 +1398,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", BTORDER_PROC, op_lefttype, op_righttype, opfamily); - inputcollation = lfirst_oid(collids_cell); - collids_cell = lnext(collids_cell); - /* * rightop is the constant or variable comparison value */ - rightop = (Expr *) lfirst(rargs_cell); - rargs_cell = lnext(rargs_cell); - if (rightop && IsA(rightop, RelabelType)) rightop = ((RelabelType *) rightop)->arg; diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index c721054787..555c91f61e 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -1680,9 +1680,7 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect, */ tlist = testlist = paramids = NIL; resno = 1; - /* there's no "forfour" so we have to chase one of the lists manually */ - cc = list_head(opcollations); - forthree(lc, leftargs, rc, rightargs, oc, opids) + forfour(lc, leftargs, rc, rightargs, oc, opids, cc, opcollations) { Node *leftarg = (Node *) lfirst(lc); Node *rightarg = (Node *) lfirst(rc); @@ -1690,7 +1688,6 @@ convert_EXISTS_to_ANY(PlannerInfo *root, Query *subselect, Oid opcollation = lfirst_oid(cc); Param *param; - cc = lnext(cc); param = generate_new_exec_param(root, exprType(rightarg), exprTypmod(rightarg), diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 55eeb5127c..eb815c2f12 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1130,17 +1130,14 @@ generate_setop_tlist(List *colTypes, List *colCollations, TargetEntry *tle; Node *expr; - /* there's no forfour() so we must chase one list manually */ - rtlc = list_head(refnames_tlist); - forthree(ctlc, colTypes, cclc, colCollations, itlc, input_tlist) + forfour(ctlc, colTypes, cclc, colCollations, + itlc, input_tlist, rtlc, refnames_tlist) { Oid colType = lfirst_oid(ctlc); Oid colColl = lfirst_oid(cclc); TargetEntry *inputtle = (TargetEntry *) lfirst(itlc); TargetEntry *reftle = (TargetEntry *) lfirst(rtlc); - rtlc = lnext(rtlc); - Assert(inputtle->resno == resno); Assert(reftle->resno == resno); Assert(!inputtle->resjunk); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index e3544efb6f..d6cdd16607 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -831,18 +831,14 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) */ rte = pstate->p_target_rangetblentry; qry->targetList = NIL; - icols = list_head(icolumns); - attnos = list_head(attrnos); - foreach(lc, exprList) + Assert(list_length(exprList) <= list_length(icolumns)); + forthree(lc, exprList, icols, icolumns, attnos, attrnos) { Expr *expr = (Expr *) lfirst(lc); - ResTarget *col; - AttrNumber attr_num; + ResTarget *col = lfirst_node(ResTarget, icols); + AttrNumber attr_num = (AttrNumber) lfirst_int(attnos); TargetEntry *tle; - col = lfirst_node(ResTarget, icols); - attr_num = (AttrNumber) lfirst_int(attnos); - tle = makeTargetEntry(expr, attr_num, col->name, @@ -851,9 +847,6 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) rte->insertedCols = bms_add_member(rte->insertedCols, attr_num - FirstLowInvalidHeapAttributeNumber); - - icols = lnext(icols); - attnos = lnext(attnos); } /* Process ON CONFLICT, if any. */ @@ -950,19 +943,16 @@ transformInsertRow(ParseState *pstate, List *exprlist, * Prepare columns for assignment to target table. */ result = NIL; - icols = list_head(icolumns); - attnos = list_head(attrnos); - foreach(lc, exprlist) + forthree(lc, exprlist, icols, icolumns, attnos, attrnos) { Expr *expr = (Expr *) lfirst(lc); - ResTarget *col; - - col = lfirst_node(ResTarget, icols); + ResTarget *col = lfirst_node(ResTarget, icols); + int attno = lfirst_int(attnos); expr = transformAssignedExpr(pstate, expr, EXPR_KIND_INSERT_TARGET, col->name, - lfirst_int(attnos), + attno, col->indirection, col->location); @@ -991,9 +981,6 @@ transformInsertRow(ParseState *pstate, List *exprlist, } result = lappend(result, expr); - - icols = lnext(icols); - attnos = lnext(attnos); } return result; @@ -1699,11 +1686,11 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->targetList = NIL; targetvars = NIL; targetnames = NIL; - left_tlist = list_head(leftmostQuery->targetList); - forthree(lct, sostmt->colTypes, - lcm, sostmt->colTypmods, - lcc, sostmt->colCollations) + forfour(lct, sostmt->colTypes, + lcm, sostmt->colTypmods, + lcc, sostmt->colCollations, + left_tlist, leftmostQuery->targetList) { Oid colType = lfirst_oid(lct); int32 colTypmod = lfirst_int(lcm); @@ -1729,7 +1716,6 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt) qry->targetList = lappend(qry->targetList, tle); targetvars = lappend(targetvars, var); targetnames = lappend(targetnames, makeString(colName)); - left_tlist = lnext(left_tlist); } /* @@ -2201,10 +2187,9 @@ determineRecursiveColTypes(ParseState *pstate, Node *larg, List *nrtargetlist) * dummy result expressions of the non-recursive term. */ targetList = NIL; - left_tlist = list_head(leftmostQuery->targetList); next_resno = 1; - foreach(nrtl, nrtargetlist) + forboth(nrtl, nrtargetlist, left_tlist, leftmostQuery->targetList) { TargetEntry *nrtle = (TargetEntry *) lfirst(nrtl); TargetEntry *lefttle = (TargetEntry *) lfirst(left_tlist); @@ -2218,7 +2203,6 @@ determineRecursiveColTypes(ParseState *pstate, Node *larg, List *nrtargetlist) colName, false); targetList = lappend(targetList, tle); - left_tlist = lnext(left_tlist); } /* Now build CTE's output column info using dummy targetlist */ diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 5222231b51..654ee80b27 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2124,13 +2124,12 @@ LookupFuncWithArgs(ObjectType objtype, ObjectWithArgs *func, bool noError) FUNC_MAX_ARGS, FUNC_MAX_ARGS))); - args_item = list_head(func->objargs); - for (i = 0; i < argcount; i++) + i = 0; + foreach(args_item, func->objargs) { TypeName *t = (TypeName *) lfirst(args_item); - argoids[i] = LookupTypeNameOid(NULL, t, noError); - args_item = lnext(args_item); + argoids[i++] = LookupTypeNameOid(NULL, t, noError); } /* diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1258092dc8..85055bbb95 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9811,31 +9811,18 @@ get_tablefunc(TableFunc *tf, deparse_context *context, bool showimplicit) ListCell *l5; int colnum = 0; - l2 = list_head(tf->coltypes); - l3 = list_head(tf->coltypmods); - l4 = list_head(tf->colexprs); - l5 = list_head(tf->coldefexprs); - appendStringInfoString(buf, " COLUMNS "); - foreach(l1, tf->colnames) + forfive(l1, tf->colnames, l2, tf->coltypes, l3, tf->coltypmods, + l4, tf->colexprs, l5, tf->coldefexprs) { char *colname = strVal(lfirst(l1)); - Oid typid; - int32 typmod; - Node *colexpr; - Node *coldefexpr; - bool ordinality = tf->ordinalitycol == colnum; + Oid typid = lfirst_oid(l2); + int32 typmod = lfirst_int(l3); + Node *colexpr = (Node *) lfirst(l4); + Node *coldefexpr = (Node *) lfirst(l5); + bool ordinality = (tf->ordinalitycol == colnum); bool notnull = bms_is_member(colnum, tf->notnulls); - typid = lfirst_oid(l2); - l2 = lnext(l2); - typmod = lfirst_int(l3); - l3 = lnext(l3); - colexpr = (Node *) lfirst(l4); - l4 = lnext(l4); - coldefexpr = (Node *) lfirst(l5); - l5 = lnext(l5); - if (colnum > 0) appendStringInfoString(buf, ", "); colnum++; @@ -10349,12 +10336,11 @@ get_from_clause_coldeflist(RangeTblFunction *rtfunc, appendStringInfoChar(buf, '('); - /* there's no forfour(), so must chase one list the hard way */ i = 0; - l4 = list_head(rtfunc->funccolnames); - forthree(l1, rtfunc->funccoltypes, - l2, rtfunc->funccoltypmods, - l3, rtfunc->funccolcollations) + forfour(l1, rtfunc->funccoltypes, + l2, rtfunc->funccoltypmods, + l3, rtfunc->funccolcollations, + l4, rtfunc->funccolnames) { Oid atttypid = lfirst_oid(l1); int32 atttypmod = lfirst_int(l2); @@ -10378,7 +10364,6 @@ get_from_clause_coldeflist(RangeTblFunction *rtfunc, appendStringInfo(buf, " COLLATE %s", generate_collation_name(attcollation)); - l4 = lnext(l4); i++; } diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h index 4624604783..8dd22e795e 100644 --- a/src/include/nodes/pg_list.h +++ b/src/include/nodes/pg_list.h @@ -205,6 +205,32 @@ list_length(const List *l) (cell1) != NULL && (cell2) != NULL && (cell3) != NULL; \ (cell1) = lnext(cell1), (cell2) = lnext(cell2), (cell3) = lnext(cell3)) +/* + * forfour - + * the same for four lists + */ +#define forfour(cell1, list1, cell2, list2, cell3, list3, cell4, list4) \ + for ((cell1) = list_head(list1), (cell2) = list_head(list2), \ + (cell3) = list_head(list3), (cell4) = list_head(list4); \ + (cell1) != NULL && (cell2) != NULL && \ + (cell3) != NULL && (cell4) != NULL; \ + (cell1) = lnext(cell1), (cell2) = lnext(cell2), \ + (cell3) = lnext(cell3), (cell4) = lnext(cell4)) + +/* + * forfive - + * the same for five lists + */ +#define forfive(cell1, list1, cell2, list2, cell3, list3, cell4, list4, cell5, list5) \ + for ((cell1) = list_head(list1), (cell2) = list_head(list2), \ + (cell3) = list_head(list3), (cell4) = list_head(list4), \ + (cell5) = list_head(list5); \ + (cell1) != NULL && (cell2) != NULL && (cell3) != NULL && \ + (cell4) != NULL && (cell5) != NULL; \ + (cell1) = lnext(cell1), (cell2) = lnext(cell2), \ + (cell3) = lnext(cell3), (cell4) = lnext(cell4), \ + (cell5) = lnext(cell5)) + extern List *lappend(List *list, void *datum); extern List *lappend_int(List *list, int datum); extern List *lappend_oid(List *list, Oid datum);