Add versions of forboth that don't need ListCell (#5856)

We've had custom versions of Postgres its `foreach` macro which with a
hidden ListCell for quite some time now. People like these custom
macros, because they are easier to use and require less boilerplate.
This adds similar custom versions of Postgres its `forboth` macro. Now
you don't need ListCells anymore when looping over two lists at the same
time.

src/backend/distributed/commands/citus_add_local_table_to_metadata.c

@@ -1106,13 +1106,10 @@ DropDefaultExpressionsAndMoveOwnedSequenceOwnerships(Oid sourceRelationId,
 	ExtractDefaultColumnsAndOwnedSequences(sourceRelationId, &columnNameList,
 										   &ownedSequenceIdList);
 
-	ListCell *columnNameCell = NULL;
-	ListCell *ownedSequenceIdCell = NULL;
-	forboth(columnNameCell, columnNameList, ownedSequenceIdCell, ownedSequenceIdList)
+	char *columnName = NULL;
+	Oid ownedSequenceId = InvalidOid;
+	forboth_ptr_oid(columnName, columnNameList, ownedSequenceId, ownedSequenceIdList)
 	{
-		char *columnName = (char *) lfirst(columnNameCell);
-		Oid ownedSequenceId = lfirst_oid(ownedSequenceIdCell);
-
 		DropDefaultColumnDefinition(sourceRelationId, columnName);
 
 		/* column might not own a sequence */

src/backend/distributed/commands/create_distributed_table.c

@@ -592,14 +592,11 @@ EnsureSequenceTypeSupported(Oid seqOid, Oid attributeTypeId, Oid ownerRelationId
 		List *dependentSequenceList = NIL;
 		GetDependentSequencesWithRelation(citusTableId, &attnumList,
 										  &dependentSequenceList, 0);
-		ListCell *attnumCell = NULL;
-		ListCell *dependentSequenceCell = NULL;
-		forboth(attnumCell, attnumList, dependentSequenceCell,
-				dependentSequenceList)
+		AttrNumber currentAttnum = InvalidAttrNumber;
+		Oid currentSeqOid = InvalidOid;
+		forboth_int_oid(currentAttnum, attnumList, currentSeqOid,
+						dependentSequenceList)
 		{
-			AttrNumber currentAttnum = lfirst_int(attnumCell);
-			Oid currentSeqOid = lfirst_oid(dependentSequenceCell);
-
 			/*
 			 * If another distributed table is using the same sequence
 			 * in one of its column defaults, make sure the types of the
@@ -675,13 +672,10 @@ static void
 EnsureDistributedSequencesHaveOneType(Oid relationId, List *dependentSequenceList,
 									  List *attnumList)
 {
-	ListCell *attnumCell = NULL;
-	ListCell *dependentSequenceCell = NULL;
-	forboth(attnumCell, attnumList, dependentSequenceCell, dependentSequenceList)
+	AttrNumber attnum = InvalidAttrNumber;
+	Oid sequenceOid = InvalidOid;
+	forboth_int_oid(attnum, attnumList, sequenceOid, dependentSequenceList)
 	{
-		AttrNumber attnum = lfirst_int(attnumCell);
-		Oid sequenceOid = lfirst_oid(dependentSequenceCell);
-
 		/*
 		 * We should make sure that the type of the column that uses
 		 * that sequence is supported

src/backend/distributed/commands/table.c

@@ -3127,13 +3127,10 @@ InterShardDDLTaskList(Oid leftRelationId, Oid rightRelationId,
 
 	List *taskList = NIL;
 
-	ListCell *leftShardCell = NULL;
-	ListCell *rightShardCell = NULL;
-	forboth(leftShardCell, leftShardList, rightShardCell, rightShardList)
+	ShardInterval *leftShardInterval = NULL;
+	ShardInterval *rightShardInterval = NULL;
+	forboth_ptr(leftShardInterval, leftShardList, rightShardInterval, rightShardList)
 	{
-		ShardInterval *leftShardInterval = (ShardInterval *) lfirst(leftShardCell);
-		ShardInterval *rightShardInterval = (ShardInterval *) lfirst(rightShardCell);
-
 		uint64 leftShardId = leftShardInterval->shardId;
 		uint64 rightShardId = rightShardInterval->shardId;
 

src/backend/distributed/metadata/metadata_sync.c

@@ -1487,13 +1487,10 @@ GetDependentSequencesWithRelation(Oid relationId, List **attnumList,
 
 	table_close(depRel, AccessShareLock);
 
-	ListCell *attrdefOidCell = NULL;
-	ListCell *attrdefAttnumCell = NULL;
-	forboth(attrdefOidCell, attrdefResult, attrdefAttnumCell, attrdefAttnumResult)
+	AttrNumber attrdefAttnum = InvalidAttrNumber;
+	Oid attrdefOid = InvalidOid;
+	forboth_int_oid(attrdefAttnum, attrdefAttnumResult, attrdefOid, attrdefResult)
 	{
-		Oid attrdefOid = lfirst_oid(attrdefOidCell);
-		AttrNumber attrdefAttnum = lfirst_int(attrdefAttnumCell);
-
 		List *sequencesFromAttrDef = GetSequencesFromAttrDef(attrdefOid);
 
 		/* to simplify and eliminate cases like "DEFAULT nextval('..') - nextval('..')" */
@@ -1689,14 +1686,10 @@ SequenceDependencyCommandList(Oid relationId)
 
 	ExtractDefaultColumnsAndOwnedSequences(relationId, &columnNameList, &sequenceIdList);
 
-	ListCell *columnNameCell = NULL;
-	ListCell *sequenceIdCell = NULL;
-
-	forboth(columnNameCell, columnNameList, sequenceIdCell, sequenceIdList)
+	char *columnName = NULL;
+	Oid sequenceId = InvalidOid;
+	forboth_ptr_oid(columnName, columnNameList, sequenceId, sequenceIdList)
 	{
-		char *columnName = lfirst(columnNameCell);
-		Oid sequenceId = lfirst_oid(sequenceIdCell);
-
 		if (!OidIsValid(sequenceId))
 		{
 			/*

src/backend/distributed/planner/insert_select_planner.c

@@ -1513,14 +1513,10 @@ InsertSelectResultIdPrefix(uint64 planId)
 static void
 RelabelTargetEntryList(List *selectTargetList, List *insertTargetList)
 {
-	ListCell *selectTargetCell = NULL;
-	ListCell *insertTargetCell = NULL;
-
-	forboth(selectTargetCell, selectTargetList, insertTargetCell, insertTargetList)
+	TargetEntry *selectTargetEntry = NULL;
+	TargetEntry *insertTargetEntry = NULL;
+	forboth_ptr(selectTargetEntry, selectTargetList, insertTargetEntry, insertTargetList)
 	{
-		TargetEntry *selectTargetEntry = lfirst(selectTargetCell);
-		TargetEntry *insertTargetEntry = lfirst(insertTargetCell);
-
 		selectTargetEntry->resname = insertTargetEntry->resname;
 	}
 }
@@ -1537,8 +1533,6 @@ static List *
 AddInsertSelectCasts(List *insertTargetList, List *selectTargetList,
 					 Oid targetRelationId)
 {
-	ListCell *insertEntryCell = NULL;
-	ListCell *selectEntryCell = NULL;
 	List *projectedEntries = NIL;
 	List *nonProjectedEntries = NIL;
 
@@ -1553,10 +1547,10 @@ AddInsertSelectCasts(List *insertTargetList, List *selectTargetList,
 	TupleDesc destTupleDescriptor = RelationGetDescr(distributedRelation);
 
 	int targetEntryIndex = 0;
-	forboth(insertEntryCell, insertTargetList, selectEntryCell, selectTargetList)
+	TargetEntry *insertEntry = NULL;
+	TargetEntry *selectEntry = NULL;
+	forboth_ptr(insertEntry, insertTargetList, selectEntry, selectTargetList)
 	{
-		TargetEntry *insertEntry = (TargetEntry *) lfirst(insertEntryCell);
-		TargetEntry *selectEntry = (TargetEntry *) lfirst(selectEntryCell);
 		Var *insertColumn = (Var *) insertEntry->expr;
 		Form_pg_attribute attr = TupleDescAttr(destTupleDescriptor,
 											   insertEntry->resno - 1);

src/backend/distributed/planner/multi_explain.c

@@ -575,8 +575,6 @@ static void
 ExplainTaskList(CitusScanState *scanState, List *taskList, ExplainState *es,
 				ParamListInfo params)
 {
-	ListCell *taskCell = NULL;
-	ListCell *remoteExplainCell = NULL;
 	List *remoteExplainList = NIL;
 
 	/* if tasks are executed, we sort them by time; unless we are on a test env */
@@ -591,10 +589,9 @@ ExplainTaskList(CitusScanState *scanState, List *taskList, ExplainState *es,
 		taskList = SortList(taskList, CompareTasksByTaskId);
 	}
 
-	foreach(taskCell, taskList)
+	Task *task = NULL;
+	foreach_ptr(task, taskList)
 	{
-		Task *task = (Task *) lfirst(taskCell);
-
 		RemoteExplainPlan *remoteExplain = RemoteExplain(task, es, params);
 		remoteExplainList = lappend(remoteExplainList, remoteExplain);
 
@@ -604,12 +601,9 @@ ExplainTaskList(CitusScanState *scanState, List *taskList, ExplainState *es,
 		}
 	}
 
-	forboth(taskCell, taskList, remoteExplainCell, remoteExplainList)
+	RemoteExplainPlan *remoteExplain = NULL;
+	forboth_ptr(task, taskList, remoteExplain, remoteExplainList)
 	{
-		Task *task = (Task *) lfirst(taskCell);
-		RemoteExplainPlan *remoteExplain =
-			(RemoteExplainPlan *) lfirst(remoteExplainCell);
-
 		ExplainTask(scanState, task, remoteExplain->placementIndex,
 					remoteExplain->explainOutputList, es);
 	}

src/backend/distributed/planner/multi_physical_planner.c

@@ -2194,11 +2194,9 @@ QueryPushdownSqlTaskList(Query *query, uint64 jobId,
 						 DeferredErrorMessage **planningError)
 {
 	List *sqlTaskList = NIL;
-	ListCell *restrictionCell = NULL;
 	uint32 taskIdIndex = 1; /* 0 is reserved for invalid taskId */
 	int shardCount = 0;
 	bool *taskRequiredForShardIndex = NULL;
-	ListCell *prunedRelationShardCell = NULL;
 
 	/* error if shards are not co-partitioned */
 	ErrorIfUnsupportedShardDistribution(query);
@@ -2216,14 +2214,13 @@ QueryPushdownSqlTaskList(Query *query, uint64 jobId,
 	int minShardOffset = 0;
 	int maxShardOffset = 0;
 
-	forboth(prunedRelationShardCell, prunedRelationShardList,
-			restrictionCell, relationRestrictionContext->relationRestrictionList)
+	RelationRestriction *relationRestriction = NULL;
+	List *prunedShardList = NULL;
+
+	forboth_ptr(prunedShardList, prunedRelationShardList,
+				relationRestriction, relationRestrictionContext->relationRestrictionList)
 	{
-		RelationRestriction *relationRestriction =
-			(RelationRestriction *) lfirst(restrictionCell);
 		Oid relationId = relationRestriction->relationId;
-		List *prunedShardList = (List *) lfirst(prunedRelationShardCell);
-		ListCell *shardIntervalCell = NULL;
 
 		CitusTableCacheEntry *cacheEntry = GetCitusTableCacheEntry(relationId);
 		if (IsCitusTableTypeCacheEntry(cacheEntry, CITUS_TABLE_WITH_NO_DIST_KEY))
@@ -2266,9 +2263,9 @@ QueryPushdownSqlTaskList(Query *query, uint64 jobId,
 			continue;
 		}
 
-		foreach(shardIntervalCell, prunedShardList)
+		ShardInterval *shardInterval = NULL;
+		foreach_ptr(shardInterval, prunedShardList)
 		{
-			ShardInterval *shardInterval = (ShardInterval *) lfirst(shardIntervalCell);
 			int shardIndex = shardInterval->shardIndex;
 
 			taskRequiredForShardIndex[shardIndex] = true;

src/backend/distributed/utils/colocation_utils.c

@@ -303,9 +303,6 @@ MarkTablesColocated(Oid sourceRelationId, Oid targetRelationId)
 void
 ErrorIfShardPlacementsNotColocated(Oid leftRelationId, Oid rightRelationId)
 {
-	ListCell *leftShardIntervalCell = NULL;
-	ListCell *rightShardIntervalCell = NULL;
-
 	/* get sorted shard interval lists for both tables */
 	List *leftShardIntervalList = LoadShardIntervalList(leftRelationId);
 	List *rightShardIntervalList = LoadShardIntervalList(rightRelationId);
@@ -329,15 +326,11 @@ ErrorIfShardPlacementsNotColocated(Oid leftRelationId, Oid rightRelationId)
 	}
 
 	/* compare shard intervals one by one */
-	forboth(leftShardIntervalCell, leftShardIntervalList,
-			rightShardIntervalCell, rightShardIntervalList)
+	ShardInterval *leftInterval = NULL;
+	ShardInterval *rightInterval = NULL;
+	forboth_ptr(leftInterval, leftShardIntervalList,
+				rightInterval, rightShardIntervalList)
 	{
-		ShardInterval *leftInterval = (ShardInterval *) lfirst(leftShardIntervalCell);
-		ShardInterval *rightInterval = (ShardInterval *) lfirst(rightShardIntervalCell);
-
-		ListCell *leftPlacementCell = NULL;
-		ListCell *rightPlacementCell = NULL;
-
 		uint64 leftShardId = leftInterval->shardId;
 		uint64 rightShardId = rightInterval->shardId;
 
@@ -373,14 +366,11 @@ ErrorIfShardPlacementsNotColocated(Oid leftRelationId, Oid rightRelationId)
 												  CompareShardPlacementsByNode);
 
 		/* compare shard placements one by one */
-		forboth(leftPlacementCell, sortedLeftPlacementList,
-				rightPlacementCell, sortedRightPlacementList)
+		ShardPlacement *leftPlacement = NULL;
+		ShardPlacement *rightPlacement = NULL;
+		forboth_ptr(leftPlacement, sortedLeftPlacementList,
+					rightPlacement, sortedRightPlacementList)
 		{
-			ShardPlacement *leftPlacement =
-				(ShardPlacement *) lfirst(leftPlacementCell);
-			ShardPlacement *rightPlacement =
-				(ShardPlacement *) lfirst(rightPlacementCell);
-
 			/*
 			 * If shard placements are on different nodes, these shard
 			 * placements are not colocated.

src/backend/distributed/worker/worker_create_or_replace.c

@@ -244,13 +244,10 @@ CompareStringList(List *list1, List *list2)
 		return false;
 	}
 
-	ListCell *cell1 = NULL;
-	ListCell *cell2 = NULL;
-	forboth(cell1, list1, cell2, list2)
+	const char *str1 = NULL;
+	const char *str2 = NULL;
+	forboth_ptr(str1, list1, str2, list2)
 	{
-		const char *str1 = lfirst(cell1);
-		const char *str2 = lfirst(cell2);
-
 		if (strcmp(str1, str2) != 0)
 		{
 			return false;

src/backend/distributed/worker/worker_merge_protocol.c

@@ -29,6 +29,7 @@
 #include "commands/copy.h"
 #include "commands/tablecmds.h"
 #include "common/string.h"
+#include "distributed/listutils.h"
 #include "distributed/metadata_cache.h"
 #include "distributed/worker_protocol.h"
 #include "distributed/version_compat.h"
@@ -436,14 +437,11 @@ List *
 ColumnDefinitionList(List *columnNameList, List *columnTypeList)
 {
 	List *columnDefinitionList = NIL;
-	ListCell *columnNameCell = NULL;
-	ListCell *columnTypeCell = NULL;
 
-	forboth(columnNameCell, columnNameList, columnTypeCell, columnTypeList)
+	const char *columnName = NULL;
+	const char *columnType = NULL;
+	forboth_ptr(columnName, columnNameList, columnType, columnTypeList)
 	{
-		const char *columnName = (const char *) lfirst(columnNameCell);
-		const char *columnType = (const char *) lfirst(columnTypeCell);
-
 		/*
 		 * We should have a SQL compatible column type declaration; we first
 		 * convert this type to PostgreSQL's type identifiers and modifiers.

src/include/distributed/listutils.h

@@ -80,6 +80,59 @@ typedef struct ListCellAndListWrapper
 		 (((var) = lfirst_oid(var ## CellDoNotUse)) || true); \
 		 var ## CellDoNotUse = lnext_compat(l, var ## CellDoNotUse))
 
+/*
+ * forboth_ptr -
+ *	  a convenience macro which loops through two lists of pointers at the same
+ *	  time, without needing a ListCell. It only needs two declared pointer
+ *	  variables to store the pointer of each of the two cells in.
+ */
+#define forboth_ptr(var1, l1, var2, l2) \
+	for (ListCell *(var1 ## CellDoNotUse) = list_head(l1), \
+		 *(var2 ## CellDoNotUse) = list_head(l2); \
+		 (var1 ## CellDoNotUse) != NULL && \
+		 (var2 ## CellDoNotUse) != NULL && \
+		 (((var1) = lfirst(var1 ## CellDoNotUse)) || true) && \
+		 (((var2) = lfirst(var2 ## CellDoNotUse)) || true); \
+		 var1 ## CellDoNotUse = lnext_compat(l1, var1 ## CellDoNotUse), \
+		 var2 ## CellDoNotUse = lnext_compat(l2, var2 ## CellDoNotUse) \
+		 )
+
+/*
+ * forboth_ptr_oid -
+ *	  a convenience macro which loops through two lists at the same time. The
+ *	  first list should contain pointers and the second list should contain
+ *	  Oids. It does not need a ListCell to do this. It only needs two declared
+ *	  variables to store the pointer and the Oid of each of the two cells in.
+ */
+#define forboth_ptr_oid(var1, l1, var2, l2) \
+	for (ListCell *(var1 ## CellDoNotUse) = list_head(l1), \
+		 *(var2 ## CellDoNotUse) = list_head(l2); \
+		 (var1 ## CellDoNotUse) != NULL && \
+		 (var2 ## CellDoNotUse) != NULL && \
+		 (((var1) = lfirst(var1 ## CellDoNotUse)) || true) && \
+		 (((var2) = lfirst_oid(var2 ## CellDoNotUse)) || true); \
+		 var1 ## CellDoNotUse = lnext_compat(l1, var1 ## CellDoNotUse), \
+		 var2 ## CellDoNotUse = lnext_compat(l2, var2 ## CellDoNotUse) \
+		 )
+
+/*
+ * forboth_int_oid -
+ *	  a convenience macro which loops through two lists at the same time. The
+ *	  first list should contain integers and the second list should contain
+ *	  Oids. It does not need a ListCell to do this. It only needs two declared
+ *	  variables to store the int and the Oid of each of the two cells in.
+ */
+#define forboth_int_oid(var1, l1, var2, l2) \
+	for (ListCell *(var1 ## CellDoNotUse) = list_head(l1), \
+		 *(var2 ## CellDoNotUse) = list_head(l2); \
+		 (var1 ## CellDoNotUse) != NULL && \
+		 (var2 ## CellDoNotUse) != NULL && \
+		 (((var1) = lfirst_int(var1 ## CellDoNotUse)) || true) && \
+		 (((var2) = lfirst_oid(var2 ## CellDoNotUse)) || true); \
+		 var1 ## CellDoNotUse = lnext_compat(l1, var1 ## CellDoNotUse), \
+		 var2 ## CellDoNotUse = lnext_compat(l2, var2 ## CellDoNotUse) \
+		 )
+
 /*
  * foreach_ptr_append -
  *	  a convenience macro which loops through a pointer List and can append list