From bb2e5f0e136755adde571bc6a4b2ab76df9f7fb6 Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Wed, 3 Nov 2021 18:29:56 +0100 Subject: [PATCH] manual work to find places where we could use new macros --- .../deparser/deparse_statistics_stmts.c | 15 ++++++------ .../distributed_intermediate_results.c | 5 +--- .../executor/insert_select_executor.c | 15 +++--------- .../distributed/executor/multi_executor.c | 13 ++++------ .../distributed/metadata/metadata_cache.c | 10 +++----- .../distributed/metadata/metadata_sync.c | 10 ++++---- .../distributed/operations/shard_rebalancer.c | 6 ++--- .../distributed/test/distribution_metadata.c | 9 +++---- src/backend/distributed/test/metadata_sync.c | 5 ++-- .../distributed/test/prune_shard_list.c | 4 +--- .../distributed_deadlock_detection.c | 2 +- .../distributed/utils/colocation_utils.c | 5 ++-- src/backend/distributed/utils/listutils.c | 16 ++++--------- src/backend/distributed/utils/resource_lock.c | 24 +++++++------------ 14 files changed, 47 insertions(+), 92 deletions(-) diff --git a/src/backend/distributed/deparser/deparse_statistics_stmts.c b/src/backend/distributed/deparser/deparse_statistics_stmts.c index 558c242df..91f04b4f3 100644 --- a/src/backend/distributed/deparser/deparse_statistics_stmts.c +++ b/src/backend/distributed/deparser/deparse_statistics_stmts.c @@ -222,12 +222,12 @@ AppendStatTypes(StringInfo buf, CreateStatsStmt *stmt) Value *statType = NULL; foreach_ptr(statType, stmt->stat_types) { - appendStringInfoString(buf, strVal(statType)); - - if (statType != llast(stmt->stat_types)) + if (!foreach_first(statType)) { appendStringInfoString(buf, ", "); } + + appendStringInfoString(buf, strVal(statType)); } appendStringInfoString(buf, ")"); @@ -250,14 +250,13 @@ AppendColumnNames(StringInfo buf, CreateStatsStmt *stmt) "only simple column references are allowed in CREATE STATISTICS"))); } - const char *columnName = quote_identifier(column->name); - - appendStringInfoString(buf, columnName); - - if (column != llast(stmt->exprs)) + if (!foreach_first(column)) { appendStringInfoString(buf, ", "); } + + const char *columnName = quote_identifier(column->name); + appendStringInfoString(buf, columnName); } } diff --git a/src/backend/distributed/executor/distributed_intermediate_results.c b/src/backend/distributed/executor/distributed_intermediate_results.c index 8a29e633d..29bb1ab3a 100644 --- a/src/backend/distributed/executor/distributed_intermediate_results.c +++ b/src/backend/distributed/executor/distributed_intermediate_results.c @@ -637,7 +637,6 @@ QueryStringForFragmentsTransfer(NodeToNodeFragmentsTransfer *fragmentsTransfer) { StringInfo queryString = makeStringInfo(); StringInfo fragmentNamesArrayString = makeStringInfo(); - int fragmentCount = 0; NodePair *nodePair = &fragmentsTransfer->nodes; WorkerNode *sourceNode = LookupNodeByNodeIdOrError(nodePair->sourceNodeId); @@ -648,15 +647,13 @@ QueryStringForFragmentsTransfer(NodeToNodeFragmentsTransfer *fragmentsTransfer) { const char *fragmentName = fragment->resultId; - if (fragmentCount > 0) + if (!foreach_first(fragment)) { appendStringInfoString(fragmentNamesArrayString, ","); } appendStringInfoString(fragmentNamesArrayString, quote_literal_cstr(fragmentName)); - - fragmentCount++; } appendStringInfoString(fragmentNamesArrayString, "]::text[]"); diff --git a/src/backend/distributed/executor/insert_select_executor.c b/src/backend/distributed/executor/insert_select_executor.c index 338b03075..783170c09 100644 --- a/src/backend/distributed/executor/insert_select_executor.c +++ b/src/backend/distributed/executor/insert_select_executor.c @@ -580,7 +580,6 @@ static int PartitionColumnIndexFromColumnList(Oid relationId, List *columnNameList) { Var *partitionColumn = PartitionColumn(relationId, 0); - int partitionColumnIndex = 0; const char *columnName = NULL; foreach_ptr(columnName, columnNameList) @@ -590,10 +589,8 @@ PartitionColumnIndexFromColumnList(Oid relationId, List *columnNameList) /* check whether this is the partition column */ if (partitionColumn != NULL && attrNumber == partitionColumn->varattno) { - return partitionColumnIndex; + return foreach_index(columnName); } - - partitionColumnIndex++; } return -1; @@ -727,15 +724,12 @@ static int PartitionColumnIndex(List *insertTargetList, Var *partitionColumn) { TargetEntry *insertTargetEntry = NULL; - int targetEntryIndex = 0; foreach_ptr(insertTargetEntry, insertTargetList) { if (insertTargetEntry->resno == partitionColumn->varattno) { - return targetEntryIndex; + return foreach_index(insertTargetEntry); } - - targetEntryIndex++; } return -1; @@ -803,11 +797,10 @@ static void WrapTaskListForProjection(List *taskList, List *projectedTargetEntries) { StringInfo projectedColumnsString = makeStringInfo(); - int entryIndex = 0; TargetEntry *targetEntry = NULL; foreach_ptr(targetEntry, projectedTargetEntries) { - if (entryIndex != 0) + if (!foreach_first(targetEntry)) { appendStringInfoChar(projectedColumnsString, ','); } @@ -815,8 +808,6 @@ WrapTaskListForProjection(List *taskList, List *projectedTargetEntries) char *columnName = targetEntry->resname; Assert(columnName != NULL); appendStringInfoString(projectedColumnsString, quote_identifier(columnName)); - - entryIndex++; } Task *task = NULL; diff --git a/src/backend/distributed/executor/multi_executor.c b/src/backend/distributed/executor/multi_executor.c index 5c36d5bba..16e01e53b 100644 --- a/src/backend/distributed/executor/multi_executor.c +++ b/src/backend/distributed/executor/multi_executor.c @@ -460,8 +460,6 @@ SortTupleStore(CitusScanState *scanState) Oid *collations = (Oid *) palloc(numberOfSortKeys * sizeof(Oid)); bool *nullsFirst = (bool *) palloc(numberOfSortKeys * sizeof(bool)); - int sortKeyIndex = 0; - /* * Iterate on the returning target list and generate the necessary information * for sorting the tuples. @@ -477,12 +475,11 @@ SortTupleStore(CitusScanState *scanState) &sortop, NULL, NULL, NULL); - sortColIdx[sortKeyIndex] = sortKeyIndex + 1; - sortOperators[sortKeyIndex] = sortop; - collations[sortKeyIndex] = exprCollation((Node *) returningEntry->expr); - nullsFirst[sortKeyIndex] = false; - - sortKeyIndex++; + int index = foreach_index(returningEntry); + sortColIdx[index] = index + 1; + sortOperators[index] = sortop; + collations[index] = exprCollation((Node *) returningEntry->expr); + nullsFirst[index] = false; } Tuplesortstate *tuplesortstate = diff --git a/src/backend/distributed/metadata/metadata_cache.c b/src/backend/distributed/metadata/metadata_cache.c index 789601d44..29b5eb886 100644 --- a/src/backend/distributed/metadata/metadata_cache.c +++ b/src/backend/distributed/metadata/metadata_cache.c @@ -1477,7 +1477,6 @@ BuildCachedShardList(CitusTableCacheEntry *cacheEntry) { Relation distShardRelation = table_open(DistShardRelationId(), AccessShareLock); TupleDesc distShardTupleDesc = RelationGetDescr(distShardRelation); - int arrayIndex = 0; shardIntervalArray = MemoryContextAllocZero(MetadataCacheMemoryContext, shardIntervalArrayLength * @@ -1501,13 +1500,12 @@ BuildCachedShardList(CitusTableCacheEntry *cacheEntry) intervalTypeMod); MemoryContext oldContext = MemoryContextSwitchTo(MetadataCacheMemoryContext); - shardIntervalArray[arrayIndex] = CopyShardInterval(shardInterval); + shardIntervalArray[foreach_index(shardTuple)] = + CopyShardInterval(shardInterval); MemoryContextSwitchTo(oldContext); heap_freetuple(shardTuple); - - arrayIndex++; } table_close(distShardRelation, AccessShareLock); @@ -1603,7 +1601,6 @@ BuildCachedShardList(CitusTableCacheEntry *cacheEntry) { ShardInterval *shardInterval = sortedShardIntervalArray[shardIndex]; int64 shardId = shardInterval->shardId; - int placementOffset = 0; /* * Enable quick lookups of this shard ID by adding it to ShardIdCacheHash @@ -1633,8 +1630,7 @@ BuildCachedShardList(CitusTableCacheEntry *cacheEntry) GroupShardPlacement *srcPlacement = NULL; foreach_ptr(srcPlacement, placementList) { - placementArray[placementOffset] = *srcPlacement; - placementOffset++; + placementArray[foreach_index(srcPlacement)] = *srcPlacement; } MemoryContextSwitchTo(oldContext); diff --git a/src/backend/distributed/metadata/metadata_sync.c b/src/backend/distributed/metadata/metadata_sync.c index ccdd2dab7..91c071ad9 100644 --- a/src/backend/distributed/metadata/metadata_sync.c +++ b/src/backend/distributed/metadata/metadata_sync.c @@ -990,6 +990,11 @@ ShardListInsertCommand(List *shardIntervalList) appendStringInfo(maxHashToken, "NULL"); } + if (!foreach_first(shardInterval)) + { + appendStringInfo(insertShardCommand, ", "); + } + appendStringInfo(insertShardCommand, "(%s::regclass, %ld, '%c'::\"char\", %s, %s)", quote_literal_cstr(qualifiedRelationName), @@ -997,11 +1002,6 @@ ShardListInsertCommand(List *shardIntervalList) shardInterval->storageType, minHashToken->data, maxHashToken->data); - - if (llast(shardIntervalList) != shardInterval) - { - appendStringInfo(insertShardCommand, ", "); - } } appendStringInfo(insertShardCommand, ") "); diff --git a/src/backend/distributed/operations/shard_rebalancer.c b/src/backend/distributed/operations/shard_rebalancer.c index 773c9e8b6..290cf3b79 100644 --- a/src/backend/distributed/operations/shard_rebalancer.c +++ b/src/backend/distributed/operations/shard_rebalancer.c @@ -280,7 +280,6 @@ CheckRebalanceStateInvariants(const RebalanceState *state) { NodeFillState *fillState = NULL; NodeFillState *prevFillState = NULL; - int fillStateIndex = 0; int fillStateLength = list_length(state->fillStateListAsc); Assert(state != NULL); @@ -300,8 +299,8 @@ CheckRebalanceStateInvariants(const RebalanceState *state) } /* Check that fillStateListDesc is the reversed version of fillStateListAsc */ - Assert(list_nth(state->fillStateListDesc, fillStateLength - fillStateIndex - 1) == - fillState); + Assert(list_nth(state->fillStateListDesc, + fillStateLength - foreach_index(fillState) - 1) == fillState); foreach_ptr(shardCost, fillState->shardCostListDesc) @@ -333,7 +332,6 @@ CheckRebalanceStateInvariants(const RebalanceState *state) 1000); prevFillState = fillState; - fillStateIndex++; } } diff --git a/src/backend/distributed/test/distribution_metadata.c b/src/backend/distributed/test/distribution_metadata.c index 97809c02f..ff455f47c 100644 --- a/src/backend/distributed/test/distribution_metadata.c +++ b/src/backend/distributed/test/distribution_metadata.c @@ -62,7 +62,6 @@ Datum load_shard_id_array(PG_FUNCTION_ARGS) { Oid distributedTableId = PG_GETARG_OID(0); - int shardIdIndex = 0; Oid shardIdTypeId = INT8OID; List *shardList = LoadShardIntervalList(distributedTableId); @@ -75,8 +74,7 @@ load_shard_id_array(PG_FUNCTION_ARGS) { Datum shardIdDatum = Int64GetDatum(shardInterval->shardId); - shardIdDatumArray[shardIdIndex] = shardIdDatum; - shardIdIndex++; + shardIdDatumArray[foreach_index(shardInterval)] = shardIdDatum; } ArrayType *shardIdArrayType = DatumArrayToArrayType(shardIdDatumArray, shardIdCount, @@ -122,7 +120,6 @@ load_shard_placement_array(PG_FUNCTION_ARGS) int64 shardId = PG_GETARG_INT64(0); bool onlyActive = PG_GETARG_BOOL(1); List *placementList = NIL; - int placementIndex = 0; Oid placementTypeId = TEXTOID; StringInfo placementInfo = makeStringInfo(); @@ -146,8 +143,8 @@ load_shard_placement_array(PG_FUNCTION_ARGS) appendStringInfo(placementInfo, "%s:%d", placement->nodeName, placement->nodePort); - placementDatumArray[placementIndex] = CStringGetTextDatum(placementInfo->data); - placementIndex++; + placementDatumArray[foreach_index(placement)] = + CStringGetTextDatum(placementInfo->data); resetStringInfo(placementInfo); } diff --git a/src/backend/distributed/test/metadata_sync.c b/src/backend/distributed/test/metadata_sync.c index 0c8622a83..5eb72d137 100644 --- a/src/backend/distributed/test/metadata_sync.c +++ b/src/backend/distributed/test/metadata_sync.c @@ -44,7 +44,6 @@ master_metadata_snapshot(PG_FUNCTION_ARGS) List *dropSnapshotCommands = MetadataDropCommands(); List *createSnapshotCommands = MetadataCreateCommands(); List *snapshotCommandList = NIL; - int snapshotCommandIndex = 0; Oid ddlCommandTypeId = TEXTOID; snapshotCommandList = list_concat(snapshotCommandList, dropSnapshotCommands); @@ -58,8 +57,8 @@ master_metadata_snapshot(PG_FUNCTION_ARGS) { Datum metadataSnapshotCommandDatum = CStringGetTextDatum(metadataSnapshotCommand); - snapshotCommandDatumArray[snapshotCommandIndex] = metadataSnapshotCommandDatum; - snapshotCommandIndex++; + snapshotCommandDatumArray[foreach_index(metadataSnapshotCommand)] = + metadataSnapshotCommandDatum; } ArrayType *snapshotCommandArrayType = DatumArrayToArrayType(snapshotCommandDatumArray, diff --git a/src/backend/distributed/test/prune_shard_list.c b/src/backend/distributed/test/prune_shard_list.c index 98aa8e7ff..9b44af848 100644 --- a/src/backend/distributed/test/prune_shard_list.c +++ b/src/backend/distributed/test/prune_shard_list.c @@ -207,7 +207,6 @@ MakeTextPartitionExpression(Oid distributedTableId, text *value) static ArrayType * PrunedShardIdsForTable(Oid distributedTableId, List *whereClauseList) { - int shardIdIndex = 0; Oid shardIdTypeId = INT8OID; Index tableId = 1; @@ -222,8 +221,7 @@ PrunedShardIdsForTable(Oid distributedTableId, List *whereClauseList) { Datum shardIdDatum = Int64GetDatum(shardInterval->shardId); - shardIdDatumArray[shardIdIndex] = shardIdDatum; - shardIdIndex++; + shardIdDatumArray[foreach_index(shardInterval)] = shardIdDatum; } ArrayType *shardIdArrayType = DatumArrayToArrayType(shardIdDatumArray, shardIdCount, diff --git a/src/backend/distributed/transaction/distributed_deadlock_detection.c b/src/backend/distributed/transaction/distributed_deadlock_detection.c index f9e4adca0..91fe56101 100644 --- a/src/backend/distributed/transaction/distributed_deadlock_detection.c +++ b/src/backend/distributed/transaction/distributed_deadlock_detection.c @@ -671,7 +671,7 @@ WaitsForToString(List *waitsFor) TransactionNode *waitingNode = NULL; foreach_ptr(waitingNode, waitsFor) { - if (transactionIdStr->len != 0) + if (!foreach_first(waitingNode)) { appendStringInfoString(transactionIdStr, ","); } diff --git a/src/backend/distributed/utils/colocation_utils.c b/src/backend/distributed/utils/colocation_utils.c index 4edc9e424..116458102 100644 --- a/src/backend/distributed/utils/colocation_utils.c +++ b/src/backend/distributed/utils/colocation_utils.c @@ -182,7 +182,6 @@ get_colocated_shard_array(PG_FUNCTION_ARGS) int colocatedShardCount = list_length(colocatedShardList); Datum *colocatedShardsDatumArray = palloc0(colocatedShardCount * sizeof(Datum)); Oid arrayTypeId = OIDOID; - int colocatedShardIndex = 0; ShardInterval *colocatedShardInterval = NULL; foreach_ptr(colocatedShardInterval, colocatedShardList) @@ -191,8 +190,8 @@ get_colocated_shard_array(PG_FUNCTION_ARGS) Datum colocatedShardDatum = Int64GetDatum(colocatedShardId); - colocatedShardsDatumArray[colocatedShardIndex] = colocatedShardDatum; - colocatedShardIndex++; + colocatedShardsDatumArray[foreach_index(colocatedShardInterval)] = + colocatedShardDatum; } ArrayType *colocatedShardsArrayType = DatumArrayToArrayType(colocatedShardsDatumArray, diff --git a/src/backend/distributed/utils/listutils.c b/src/backend/distributed/utils/listutils.c index 836a4bff6..862da2895 100644 --- a/src/backend/distributed/utils/listutils.c +++ b/src/backend/distributed/utils/listutils.c @@ -43,9 +43,7 @@ SortList(List *pointerList, int (*comparisonFunction)(const void *, const void * void *pointer = NULL; foreach_ptr(pointer, pointerList) { - array[arrayIndex] = pointer; - - arrayIndex++; + array[foreach_index(pointer)] = pointer; } /* sort the array of pointers using the comparison function */ @@ -77,13 +75,11 @@ PointerArrayFromList(List *pointerList) { int pointerCount = list_length(pointerList); void **pointerArray = (void **) palloc0(pointerCount * sizeof(void *)); - int pointerIndex = 0; void *pointer = NULL; foreach_ptr(pointer, pointerList) { - pointerArray[pointerIndex] = pointer; - pointerIndex += 1; + pointerArray[foreach_index(pointer)] = pointer; } return pointerArray; @@ -188,15 +184,13 @@ StringJoin(List *stringList, char delimiter) StringInfo joinedString = makeStringInfo(); const char *command = NULL; - int curIndex = 0; foreach_ptr(command, stringList) { - if (curIndex > 0) + if (!foreach_first(command)) { appendStringInfoChar(joinedString, delimiter); } appendStringInfoString(joinedString, command); - curIndex++; } return joinedString->data; @@ -212,14 +206,12 @@ List * ListTake(List *pointerList, int size) { List *result = NIL; - int listIndex = 0; void *pointer = NULL; foreach_ptr(pointer, pointerList) { result = lappend(result, pointer); - listIndex++; - if (listIndex >= size) + if (list_length(result) >= size) { break; } diff --git a/src/backend/distributed/utils/resource_lock.c b/src/backend/distributed/utils/resource_lock.c index 885ce39ac..ea4be599e 100644 --- a/src/backend/distributed/utils/resource_lock.c +++ b/src/backend/distributed/utils/resource_lock.c @@ -216,8 +216,6 @@ static void LockShardListResourcesOnFirstWorker(LOCKMODE lockmode, List *shardIntervalList) { StringInfo lockCommand = makeStringInfo(); - int processedShardIntervalCount = 0; - int totalShardIntervalCount = list_length(shardIntervalList); WorkerNode *firstWorkerNode = GetFirstPrimaryWorkerNode(); int connectionFlags = 0; const char *superuser = CurrentUserName(); @@ -227,15 +225,13 @@ LockShardListResourcesOnFirstWorker(LOCKMODE lockmode, List *shardIntervalList) ShardInterval *shardInterval = NULL; foreach_ptr(shardInterval, shardIntervalList) { - int64 shardId = shardInterval->shardId; - - appendStringInfo(lockCommand, "%lu", shardId); - - processedShardIntervalCount++; - if (processedShardIntervalCount != totalShardIntervalCount) + if (!foreach_first(shardInterval)) { appendStringInfo(lockCommand, ", "); } + + int64 shardId = shardInterval->shardId; + appendStringInfo(lockCommand, "%lu", shardId); } appendStringInfo(lockCommand, "])"); @@ -303,8 +299,6 @@ void LockShardListMetadataOnWorkers(LOCKMODE lockmode, List *shardIntervalList) { StringInfo lockCommand = makeStringInfo(); - int processedShardIntervalCount = 0; - int totalShardIntervalCount = list_length(shardIntervalList); if (list_length(shardIntervalList) == 0) { @@ -316,15 +310,13 @@ LockShardListMetadataOnWorkers(LOCKMODE lockmode, List *shardIntervalList) ShardInterval *shardInterval = NULL; foreach_ptr(shardInterval, shardIntervalList) { - int64 shardId = shardInterval->shardId; - - appendStringInfo(lockCommand, "%lu", shardId); - - processedShardIntervalCount++; - if (processedShardIntervalCount != totalShardIntervalCount) + if (!foreach_first(shardInterval)) { appendStringInfo(lockCommand, ", "); } + + int64 shardId = shardInterval->shardId; + appendStringInfo(lockCommand, "%lu", shardId); } appendStringInfo(lockCommand, "])");