From a5010e5b17e8db5f45810e8b81ac8250492c8b0f Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Wed, 23 Oct 2019 16:49:12 +0200 Subject: [PATCH] Add extra foreach convenience macros (#3117) This completely hides `ListCell` to the user of the loop Example usage: ```c WorkerNode *workerNode = NULL; foreach_ptr(workerNode, workerNodeList) { // Do stuff with workerNode } ``` Instead of: ```c ListCell *workerNodeCell = NULL; foreach(cell, workerNodeList) { WorkerNode *workerNode = lfirst(workerNodeCell); // Do stuff with workerNode } ``` --- src/backend/distributed/commands/truncate.c | 11 ++--- .../planner/query_pushdown_planning.c | 6 +-- src/backend/distributed/utils/node_metadata.c | 5 +-- src/include/distributed/listutils.h | 45 +++++++++++++++++++ 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/backend/distributed/commands/truncate.c b/src/backend/distributed/commands/truncate.c index 53052b52e..1a54f11a9 100644 --- a/src/backend/distributed/commands/truncate.c +++ b/src/backend/distributed/commands/truncate.c @@ -181,7 +181,7 @@ LockTruncatedRelationMetadataInWorkers(TruncateStmt *truncateStatement) Oid relationId = RangeVarGetRelid(rangeVar, NoLock, false); DistTableCacheEntry *cacheEntry = NULL; List *referencingTableList = NIL; - ListCell *referencingTableCell = NULL; + Oid referencingRelationId = InvalidOid; if (!IsDistributedTable(relationId)) { @@ -199,9 +199,8 @@ LockTruncatedRelationMetadataInWorkers(TruncateStmt *truncateStatement) Assert(cacheEntry != NULL); referencingTableList = cacheEntry->referencingRelationsViaForeignKey; - foreach(referencingTableCell, referencingTableList) + foreach_oid(referencingRelationId, referencingTableList) { - Oid referencingRelationId = lfirst_oid(referencingTableCell); distributedRelationList = list_append_unique_oid(distributedRelationList, referencingRelationId); } @@ -228,7 +227,7 @@ LockTruncatedRelationMetadataInWorkers(TruncateStmt *truncateStatement) static void AcquireDistributedLockOnRelations(List *relationIdList, LOCKMODE lockMode) { - ListCell *relationIdCell = NULL; + Oid relationId = InvalidOid; List *workerNodeList = ActivePrimaryNodeList(NoLock); const char *lockModeText = LockModeToLockModeText(lockMode); @@ -241,10 +240,8 @@ AcquireDistributedLockOnRelations(List *relationIdList, LOCKMODE lockMode) BeginOrContinueCoordinatedTransaction(); - foreach(relationIdCell, relationIdList) + foreach_oid(relationId, relationIdList) { - Oid relationId = lfirst_oid(relationIdCell); - /* * We only acquire distributed lock on relation if * the relation is sync'ed between mx nodes. diff --git a/src/backend/distributed/planner/query_pushdown_planning.c b/src/backend/distributed/planner/query_pushdown_planning.c index 7e8565400..3cc55325e 100644 --- a/src/backend/distributed/planner/query_pushdown_planning.c +++ b/src/backend/distributed/planner/query_pushdown_planning.c @@ -24,6 +24,7 @@ #include "distributed/citus_clauses.h" #include "distributed/citus_ruleutils.h" #include "distributed/deparse_shard_query.h" +#include "distributed/listutils.h" #include "distributed/metadata_cache.h" #include "distributed/multi_logical_optimizer.h" #include "distributed/multi_logical_planner.h" @@ -1061,7 +1062,7 @@ DeferErrorIfUnsupportedTableCombination(Query *queryTree) { List *rangeTableList = queryTree->rtable; List *joinTreeTableIndexList = NIL; - ListCell *joinTreeTableIndexCell = NULL; + int joinTreeTableIndex = 0; bool unsupportedTableCombination = false; char *errorDetail = NULL; @@ -1071,13 +1072,12 @@ DeferErrorIfUnsupportedTableCombination(Query *queryTree) */ ExtractRangeTableIndexWalker((Node *) queryTree->jointree, &joinTreeTableIndexList); - foreach(joinTreeTableIndexCell, joinTreeTableIndexList) + foreach_int(joinTreeTableIndex, joinTreeTableIndexList) { /* * Join tree's range table index starts from 1 in the query tree. But, * list indexes start from 0. */ - int joinTreeTableIndex = lfirst_int(joinTreeTableIndexCell); int rangeTableListIndex = joinTreeTableIndex - 1; RangeTblEntry *rangeTableEntry = diff --git a/src/backend/distributed/utils/node_metadata.c b/src/backend/distributed/utils/node_metadata.c index 049a517d8..d59570b88 100644 --- a/src/backend/distributed/utils/node_metadata.c +++ b/src/backend/distributed/utils/node_metadata.c @@ -765,8 +765,8 @@ UpdateNodeLocation(int32 nodeId, char *newNodeName, int32 newNodePort) Datum master_initialize_node_metadata(PG_FUNCTION_ARGS) { - ListCell *workerNodeCell = NULL; List *workerNodes = NIL; + WorkerNode *workerNode = NULL; CheckCitusVersion(ERROR); @@ -779,9 +779,8 @@ master_initialize_node_metadata(PG_FUNCTION_ARGS) workerNodes = ParseWorkerNodeFileAndRename(); - foreach(workerNodeCell, workerNodes) + foreach_ptr(workerNode, workerNodes) { - WorkerNode *workerNode = (WorkerNode *) lfirst(workerNodeCell); bool nodeAlreadyExists = false; NodeMetadata nodeMetadata = DefaultNodeMetadata(); nodeMetadata.nodeRack = workerNode->workerRack; diff --git a/src/include/distributed/listutils.h b/src/include/distributed/listutils.h index 22f8b65d6..bb2191ec8 100644 --- a/src/include/distributed/listutils.h +++ b/src/include/distributed/listutils.h @@ -20,6 +20,51 @@ #include "utils/hsearch.h" +/* + * foreach_ptr - + * a convenience macro which loops through a pointer list without needing a + * ListCell, just a declared pointer variable to store the pointer of the + * cell in. + * + * How it works: + * - A ListCell is declared with the name {var}Cell and used throughout the + * for loop using ## to concat. + * - To assign to var it needs to be done in the condition of the for loop, + * because we cannot use the initializer since a ListCell* variable is + * declared there. + * - || true is used to always enter the loop when cell is not null even if + * var is NULL. + */ +#define foreach_ptr(var, l) \ + for (ListCell *(var ## Cell) = list_head(l); \ + (var ## Cell) != NULL && (((var) = lfirst(var ## Cell)) || true); \ + var ## Cell = lnext(var ## Cell)) + + +/* + * foreach_int - + * a convenience macro which loops through an int list without needing a + * ListCell, just a declared int variable to store the int of the cell in. + * For explanation of how it works see foreach_ptr. + */ +#define foreach_int(var, l) \ + for (ListCell *(var ## Cell) = list_head(l); \ + (var ## Cell) != NULL && (((var) = lfirst_int(var ## Cell)) || true); \ + var ## Cell = lnext(var ## Cell)) + + +/* + * foreach_oid - + * a convenience macro which loops through an oid list without needing a + * ListCell, just a declared Oid variable to store the oid of the cell in. + * For explanation of how it works see foreach_ptr. + */ +#define foreach_oid(var, l) \ + for (ListCell *(var ## Cell) = list_head(l); \ + (var ## Cell) != NULL && (((var) = lfirst_oid(var ## Cell)) || true); \ + var ## Cell = lnext(var ## Cell)) + + /* utility functions declaration shared within this module */ extern List * SortList(List *pointerList, int (*ComparisonFunction)(const void *, const void *));