Attached is a slightly updated version, with a bit simpler
implementation of foreach_delete_current.
Instead of decrementing i and then adding 1 to it when indexing the
list, it now indexes the list using a postfix decrement.
From 9073777a6d82e2b2db8b1ed9aef200550234d89a 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 v4 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
From 4f5bbe8865a420d635d0d4357388d1c3632a9821 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Tue, 24 Oct 2023 16:46:08 +0200
Subject: [PATCH v4 1/2] Add macros for looping through a list without needing
a ListCell
Many usages of the foreach macro only use the ListCell variable to get
its contents. This adds macros that simplify iteration code for that
common use case.
---
src/include/nodes/pg_list.h | 111 ++++++++++++++++++++++++++++++++++--
1 file changed, 105 insertions(+), 6 deletions(-)
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 529a382d284..e09116e8fdb 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -383,13 +383,14 @@ lnext(const List *l, const ListCell *c)
* delete the current list element from the List associated with a
* surrounding foreach() loop, returning the new List pointer.
*
- * This is equivalent to list_delete_cell(), but it also adjusts the foreach
- * loop's state so that no list elements will be missed. Do not delete
- * elements from an active foreach loop's list in any other way!
+ * This is similar to list_delete_cell(), but it also works when using
+ * for_each_xyz macros that don't require passing in a "ListCell *".
+ * Furthermore it adjusts the foreach loop's state so that no list elements
+ * will be missed. Do not delete elements from an active foreach loop's list in
+ * any other way!
*/
-#define foreach_delete_current(lst, cell) \
- (cell##__state.i--, \
- (List *) (cell##__state.l = list_delete_cell(lst, cell)))
+#define foreach_delete_current(lst, var) \
+ ((List *) (var##__state.l = list_delete_cell(lst, &var##__state.l->elements[var##__state.i--])))
/*
* foreach_current_index -
@@ -452,6 +453,104 @@ for_each_cell_setup(const List *lst, const ListCell *initcell)
return r;
}
+/*
+ * for_each_ptr -
+ * a convenience macro which loops through a list of pointers without
+ * needing a "ListCell *", just a declared pointer variable to store the
+ * current pointer int;
+ *
+ * Unlike with foreach() it's not possible to detect if an early "break"
+ * occured by checking the value of the loop variable at the end of the loop.
+ * If you need this, it's recommended to use foreach() instead or manually keep
+ * track of a break occured using a boolean flag variable called e.g. "found".
+ *
+ * The caveats for foreach() apply equally here.
+ */
+#define for_each_ptr(var, lst) \
+ for (ForEachState var##__state = {(lst), 0}; \
+ (var##__state.l != NIL && \
+ var##__state.i < var##__state.l->length && \
+ (var = lfirst(&var##__state.l->elements[var##__state.i]), true));\
+ var##__state.i++)
+
+/*
+ * for_each_int -
+ * a convenience macro which loops through a list of ints without needing a
+ * "ListCell *", just a declared int variable to store the current int in.
+ *
+ * Unlike with foreach() it's not possible to detect if an early "break"
+ * occured by checking the value of the loop variable at the end of the loop.
+ * If you need this, it's recommended to use foreach() instead or manually keep
+ * track of a break occured using a boolean flag variable called e.g. "found".
+ *
+ * The caveats for foreach() apply equally here.
+ */
+#define for_each_int(var, lst) \
+ for (ForEachState var##__state = {(lst), 0}; \
+ (var##__state.l != NIL && \
+ var##__state.i < var##__state.l->length && \
+ (var = lfirst_int(&var##__state.l->elements[var##__state.i]), true));\
+ var##__state.i++)
+
+/*
+ * foreach_oid -
+ * a convenience macro which loops through an oid list without needing a
+ * "ListCell *", just a declared Oid variable to store the current oid in.
+ *
+ * Unlike with foreach() it's not possible to detect if an early "break"
+ * occured by checking the value of the loop variable at the end of the loop.
+ * If you need this, it's recommended to use foreach() instead or manually keep
+ * track of a break occured using a boolean flag variable called e.g. "found".
+ *
+ * The caveats for foreach() apply equally here.
+ */
+#define for_each_oid(var, lst) \
+ for (ForEachState var##__state = {(lst), 0}; \
+ (var##__state.l != NIL && \
+ var##__state.i < var##__state.l->length && \
+ (var = lfirst_oid(&var##__state.l->elements[var##__state.i]), true));\
+ var##__state.i++)
+
+/*
+ * foreach_xid -
+ * a convenience macro which loops through an xid list without needing a
+ * "ListCell *", just a declared TransactionId variable to store the current
+ * xid in.
+ *
+ * Unlike with foreach() it's not possible to detect if an early "break"
+ * occured by checking the value of the loop variable at the end of the loop.
+ * If you need this, it's recommended to use foreach() instead or manually keep
+ * track of a break occured using a boolean flag variable called e.g. "found".
+ *
+ * The caveats for foreach() apply equally here.
+ */
+#define for_each_xid(var, lst) \
+ for (ForEachState var##__state = {(lst), 0}; \
+ (var##__state.l != NIL && \
+ var##__state.i < var##__state.l->length && \
+ (var = lfirst_xid(&var##__state.l->elements[var##__state.i]), true));\
+ var##__state.i++)
+
+/*
+ * for_each_node -
+ * a convenience macro which loops through a node list without needing a
+ * "ListCell *", just a declared pointer variable to store the pointer of
+ * the current node in.
+ *
+ * Unlike with foreach() it's not possible to detect if an early "break"
+ * occured by checking the value of the loop variable at the end of the loop.
+ * If you need this, it's recommended to use foreach() instead or manually keep
+ * track of a break occured using a boolean flag variable called e.g. "found".
+ *
+ * The caveats for foreach() apply equally here.
+ */
+#define for_each_node(type, var, lst) \
+ for (ForEachState var##__state = {(lst), 0}; \
+ (var##__state.l != NIL && \
+ var##__state.i < var##__state.l->length && \
+ (var = lfirst_node(type, &var##__state.l->elements[var##__state.i]), true));\
+ var##__state.i++)
+
/*
* forboth -
* a convenience macro for advancing through two linked lists
base-commit: 8f0fd47fa33720dd09ad0ae74a8a583b9780e328
--
2.34.1
