Long Functions and Long Files

After having the opportunity to see some parts of the codebase, I would like to make 2 suggestions regarding long functions and files. We have some very long functions and files:

### Long Functions

We have a style checker but it doesnt check function complexities. I think it would be cool to have a tool that checks function complexities ( Like if we have very nested if else etc). When we have long functions, it is hard to follow what a function does in a high level. However, when we replace the lines that make a logical block with a function, it is easier to see the high level and we can go to a function to see the details of what is done.

For example a random method:

old method

```c

static List *MapTaskList(MapMergeJob *mapMergeJob, List *filterTaskList) {

List *mapTaskList = NIL;
Query *filterQuery = mapMergeJob->job.jobQuery;
List *rangeTableList = filterQuery->rtable;
ListCell *filterTaskCell = NULL;
Var *partitionColumn = mapMergeJob->partitionColumn;
Oid partitionColumnType = partitionColumn->vartype;
char *partitionColumnTypeFullName = format_type_be_qualified(partitionColumnType);
int32 partitionColumnTypeMod = partitionColumn->vartypmod;
char *partitionColumnName = NULL;

List *groupClauseList = filterQuery->groupClause;
if (groupClauseList != NIL)
{
List *targetEntryList = filterQuery->targetList;
List *groupTargetEntryList = GroupTargetEntryList(groupClauseList,
targetEntryList);
TargetEntry *groupByTargetEntry = (TargetEntry *) linitial(groupTargetEntryList);

partitionColumnName = groupByTargetEntry->resname;
}
else
{
partitionColumnName = ColumnName(partitionColumn, rangeTableList);
}

foreach(filterTaskCell, filterTaskList)
{
Task *filterTask = (Task *) lfirst(filterTaskCell);
uint64 jobId = filterTask->jobId;
uint32 taskId = filterTask->taskId;
Task *mapTask = NULL;

/* wrap repartition query string around filter query string */
StringInfo mapQueryString = makeStringInfo();
char *filterQueryString = filterTask->queryString;
char *filterQueryEscapedText = quote_literal_cstr(filterQueryString);

PartitionType partitionType = mapMergeJob->partitionType;
if (partitionType == RANGE_PARTITION_TYPE)
{
ShardInterval **intervalArray = mapMergeJob->sortedShardIntervalArray;
uint32 intervalCount = mapMergeJob->partitionCount;

ArrayType *splitPointObject = SplitPointObject(intervalArray, intervalCount);
StringInfo splitPointString = SplitPointArrayString(splitPointObject,
partitionColumnType,
partitionColumnTypeMod);

appendStringInfo(mapQueryString, RANGE_PARTITION_COMMAND, jobId, taskId,
filterQueryEscapedText, partitionColumnName,
partitionColumnTypeFullName, splitPointString->data);
}
else if (partitionType == SINGLE_HASH_PARTITION_TYPE)
{
ShardInterval **intervalArray = mapMergeJob->sortedShardIntervalArray;
uint32 intervalCount = mapMergeJob->partitionCount;

ArrayType *splitPointObject = SplitPointObject(intervalArray, intervalCount);
StringInfo splitPointString = SplitPointArrayString(splitPointObject,
partitionColumnType,
partitionColumnTypeMod);
appendStringInfo(mapQueryString, HASH_PARTITION_COMMAND, jobId, taskId,
filterQueryEscapedText, partitionColumnName,
partitionColumnTypeFullName, splitPointString->data);
}
else
{
uint32 partitionCount = mapMergeJob->partitionCount;
ShardInterval **intervalArray =
GenerateSyntheticShardIntervalArray(partitionCount);
ArrayType *splitPointObject = SplitPointObject(intervalArray,
mapMergeJob->partitionCount);
StringInfo splitPointString =
SplitPointArrayString(splitPointObject, INT4OID, get_typmodin(INT4OID));

appendStringInfo(mapQueryString, HASH_PARTITION_COMMAND, jobId, taskId,
filterQueryEscapedText, partitionColumnName,
partitionColumnTypeFullName, splitPointString->data);
}

/* convert filter query task into map task */
mapTask = filterTask;
mapTask->queryString = mapQueryString->data;
mapTask->taskType = MAP_TASK;

mapTaskList = lappend(mapTaskList, mapTask);
}

return mapTaskList;
}
```

can be refactored as (This is like a pseudocode):

new method

```c
static List *MapTaskList(MapMergeJob *mapMergeJob, List *filterTaskList) {
List *mapTaskList = NIL;
Query *filterQuery = mapMergeJob->job.jobQuery;
List *rangeTableList = filterQuery->rtable;
ListCell *filterTaskCell = NULL;
Var *partitionColumn = mapMergeJob->partitionColumn;
Oid partitionColumnType = partitionColumn->vartype;
char *partitionColumnTypeFullName = format_type_be_qualified(partitionColumnType);
int32 partitionColumnTypeMod = partitionColumn->vartypmod;
char *partitionColumnName = --FindPartitionColumnName()--;

foreach(filterTaskCell, filterTaskList)
{
Task *filterTask = (Task *) lfirst(filterTaskCell);

--HandlePartitionType(filterTask, mapMergeJob->partitionType);--

mapTask = **ConvertFilterQueryIntoMapTask()**;
mapTaskList = lappend(mapTaskList, mapTask);
}

return mapTaskList;
}

static void HandlePartitionType(Task *filterTask, PartitionType partitionType){
uint64 jobId = filterTask->jobId;
uint32 taskId = filterTask->taskId;

/* wrap repartition query string around filter query string */
StringInfo mapQueryString = makeStringInfo();
char *filterQueryString = filterTask->queryString;
char *filterQueryEscapedText = quote_literal_cstr(filterQueryString);

if (partitionType == RANGE_PARTITION_TYPE)
{
--HandleRangePartitionType();--
}
else if (partitionType == SINGLE_HASH_PARTITION_TYPE)
{
--HandleSingleHashPartitionType();--
}
else
{
--HandleGenericPartitionType();--
}

}
```

I believe there is no limit so as to how many lines a function should be at maximum, but it should be easy to follow the function. However 100 lines for a function will probably be almost always too long. When we see a logical block, it is an opportunity to create a function. With more functions, we also prevent duplicated code. We have a more self documented code, which is easier to follow.

### Long Files

Number of lines sorted in descending order:
```bash
find . -name "*.c" -type f -exec wc -l {} + | sort -rn | head -20
126357 total
7955 ./src/backend/distributed/utils/ruleutils_12.c
7954 ./src/backend/distributed/utils/ruleutils_11.c
5641 ./src/backend/distributed/planner/multi_physical_planner.c
4293 ./src/backend/distributed/planner/multi_logical_optimizer.c
4077 ./src/backend/distributed/utils/metadata_cache.c
3729 ./src/backend/distributed/executor/adaptive_executor.c
3495 ./src/backend/distributed/commands/multi_copy.c
3204 ./src/backend/distributed/planner/multi_router_planner.c
3036 ./src/backend/distributed/executor/multi_task_tracker_executor.c
2306 ./src/backend/distributed/planner/multi_logical_planner.c
2105 ./src/backend/distributed/executor/multi_router_executor.c
2014 ./src/backend/distributed/planner/relation_restriction_equivalence.c
1919 ./src/backend/distributed/planner/query_pushdown_planning.c
1814 ./src/backend/distributed/utils/node_metadata.c
1765 ./src/backend/distributed/planner/distributed_planner.c
1755 ./src/backend/distributed/planner/recursive_planning.c
1565 ./src/backend/distributed/planner/shard_pruning.c
1470 ./src/backend/distributed/planner/insert_select_planner.c
1466 ./src/backend/distributed/master/master_metadata_utility.c
```

With very long files, there is a lot of logic in a single file, so it is harder to follow the code, or find something. I think we should separate very long files into multiple files so that it is more clear what each file contains. There is certainly not something like X lines is too much but when a file contains tens of methods, it is just hard to follow.

I think we can have more folders. For example for each executor we can have a folder.

```
./executor/adaptive_executor/
./executor/adaptive_executor/x.c
./executor/adaptive_executor/y.c
./executor/adaptive_executor/z.c
./executor/task_tracker/
./executor/task_tracker/x.c
./executor/task_tracker/y.c
./executor/task_tracker/z.c
```

I wonder what people think about this?