From b295b8779a4c65d3c9f9b489264d4cbac6ac50db Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 24 Oct 2023 17:13:20 +0200
Subject: [PATCH v2 2/2] Use new for_each_xyz macros in a few places

This starts using each of the newly added for_each_xyz macros in at
least one place. This shows how they improve readability by reducing the
amount of boilerplate code.
---
 src/backend/executor/execExpr.c             | 11 ++++----
 src/backend/executor/nodeSubplan.c          | 28 +++++++++------------
 src/backend/replication/logical/relation.c  |  5 ++--
 src/backend/replication/logical/tablesync.c |  6 ++---
 src/backend/replication/pgoutput/pgoutput.c |  8 +++---
 5 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2c62b0c9c84..e73261ec445 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -216,7 +216,8 @@ ExecInitQual(List *qual, PlanState *parent)
 	ExprState  *state;
 	ExprEvalStep scratch = {0};
 	List	   *adjust_jumps = NIL;
-	ListCell   *lc;
+	Expr	   *node;
+	int			jump;
 
 	/* short-circuit (here and in ExecQual) for empty restriction list */
 	if (qual == NIL)
@@ -250,10 +251,8 @@ ExecInitQual(List *qual, PlanState *parent)
 	scratch.resvalue = &state->resvalue;
 	scratch.resnull = &state->resnull;
 
-	foreach(lc, qual)
+	for_each_ptr(node, qual)
 	{
-		Expr	   *node = (Expr *) lfirst(lc);
-
 		/* first evaluate expression */
 		ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
 
@@ -265,9 +264,9 @@ ExecInitQual(List *qual, PlanState *parent)
 	}
 
 	/* adjust jump targets */
-	foreach(lc, adjust_jumps)
+	for_each_int(jump, adjust_jumps)
 	{
-		ExprEvalStep *as = &state->steps[lfirst_int(lc)];
+		ExprEvalStep *as = &state->steps[jump];
 
 		Assert(as->opcode == EEOP_QUAL);
 		Assert(as->d.qualexpr.jumpdone == -1);
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index c136f75ac24..8f88f289269 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -307,7 +307,7 @@ ExecScanSubPlan(SubPlanState *node,
 		Datum		rowresult;
 		bool		rownull;
 		int			col;
-		ListCell   *plst;
+		int			paramid;
 
 		if (subLinkType == EXISTS_SUBLINK)
 		{
@@ -367,9 +367,8 @@ ExecScanSubPlan(SubPlanState *node,
 			 * Now set all the setParam params from the columns of the tuple
 			 */
 			col = 1;
-			foreach(plst, subplan->setParam)
+			for_each_int(paramid, subplan->setParam)
 			{
-				int			paramid = lfirst_int(plst);
 				ParamExecData *prmdata;
 
 				prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
@@ -412,9 +411,8 @@ ExecScanSubPlan(SubPlanState *node,
 		 * combining expression.
 		 */
 		col = 1;
-		foreach(plst, subplan->paramIds)
+		for_each_int(paramid, subplan->paramIds)
 		{
-			int			paramid = lfirst_int(plst);
 			ParamExecData *prmdata;
 
 			prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
@@ -480,10 +478,11 @@ ExecScanSubPlan(SubPlanState *node,
 		}
 		else if (subLinkType == MULTIEXPR_SUBLINK)
 		{
+			int			paramid;
+
 			/* We don't care about function result, but set the setParams */
-			foreach(l, subplan->setParam)
+			for_each_int(paramid, subplan->setParam)
 			{
-				int			paramid = lfirst_int(l);
 				ParamExecData *prmdata;
 
 				prmdata = &(econtext->ecxt_param_exec_vals[paramid]);
@@ -604,16 +603,15 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
 		 slot = ExecProcNode(planstate))
 	{
 		int			col = 1;
-		ListCell   *plst;
 		bool		isnew;
+		int			paramid;
 
 		/*
 		 * Load up the Params representing the raw sub-select outputs, then
 		 * form the projection tuple to store in the hashtable.
 		 */
-		foreach(plst, subplan->paramIds)
+		for_each_int(paramid, subplan->paramIds)
 		{
-			int			paramid = lfirst_int(plst);
 			ParamExecData *prmdata;
 
 			prmdata = &(innerecontext->ecxt_param_exec_vals[paramid]);
@@ -880,11 +878,10 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 	if (subplan->setParam != NIL && subplan->parParam == NIL &&
 		subplan->subLinkType != CTE_SUBLINK)
 	{
-		ListCell   *lst;
+		int			paramid;
 
-		foreach(lst, subplan->setParam)
+		for_each_int(paramid, subplan->setParam)
 		{
-			int			paramid = lfirst_int(lst);
 			ParamExecData *prm = &(estate->es_param_exec_vals[paramid]);
 
 			prm->execPlan = sstate;
@@ -906,7 +903,7 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 		List	   *oplist,
 				   *lefttlist,
 				   *righttlist;
-		ListCell   *l;
+		OpExpr	   *opexpr;
 
 		/* We need a memory context to hold the hash table(s) */
 		sstate->hashtablecxt =
@@ -966,9 +963,8 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
 		cross_eq_funcoids = (Oid *) palloc(ncols * sizeof(Oid));
 
 		i = 1;
-		foreach(l, oplist)
+		for_each_node(OpExpr, opexpr, oplist)
 		{
-			OpExpr	   *opexpr = lfirst_node(OpExpr, l);
 			Expr	   *expr;
 			TargetEntry *tle;
 			Oid			rhs_eq_oper;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index d62eefed138..f8faceb57ce 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -747,11 +747,10 @@ static Oid
 FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 {
 	List	   *idxlist = RelationGetIndexList(localrel);
-	ListCell   *lc;
+	Oid			idxoid;
 
-	foreach(lc, idxlist)
+	for_each_oid(idxoid, idxlist)
 	{
-		Oid			idxoid = lfirst_oid(lc);
 		bool		isUsableIdx;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 37a0abe2f4d..c82d05a642e 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -416,7 +416,7 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
 		TimestampTz last_start_time;
 	};
 	static HTAB *last_start_times = NULL;
-	ListCell   *lc;
+	SubscriptionRelState *rstate;
 	bool		started_tx = false;
 	bool		should_exit = false;
 
@@ -453,10 +453,8 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
 	/*
 	 * Process all tables that are being synchronized.
 	 */
-	foreach(lc, table_states_not_ready)
+	for_each_ptr(rstate, table_states_not_ready)
 	{
-		SubscriptionRelState *rstate = (SubscriptionRelState *) lfirst(lc);
-
 		if (rstate->state == SUBREL_STATE_SYNCDONE)
 		{
 			/*
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index c1c66848f36..ec6df59bedc 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -2229,7 +2229,7 @@ cleanup_rel_sync_cache(TransactionId xid, bool is_commit)
 {
 	HASH_SEQ_STATUS hash_seq;
 	RelationSyncEntry *entry;
-	ListCell   *lc;
+	TransactionId streamed_txn;
 
 	Assert(RelationSyncCache != NULL);
 
@@ -2242,15 +2242,15 @@ cleanup_rel_sync_cache(TransactionId xid, bool is_commit)
 		 * corresponding schema and we don't need to send it unless there is
 		 * any invalidation for that relation.
 		 */
-		foreach(lc, entry->streamed_txns)
+		for_each_xid(streamed_txn, entry->streamed_txns)
 		{
-			if (xid == lfirst_xid(lc))
+			if (xid == streamed_txn)
 			{
 				if (is_commit)
 					entry->schema_sent = true;
 
 				entry->streamed_txns =
-					foreach_delete_current(entry->streamed_txns, lc);
+					foreach_delete_current(entry->streamed_txns, streamed_txn);
 				break;
 			}
 		}
-- 
2.34.1

