Hi,
Very often we allocate a structure with palloc0() and then initialize
its fields a second time with 0 / NULL / false / etc. For instance, in
toasting.c we have:
```
indexInfo = makeNode(IndexInfo); // uses palloc0()
indexInfo->ii_NumIndexAttrs = 2;
indexInfo->ii_NumIndexKeyAttrs = 2;
indexInfo->ii_IndexAttrNumbers[0] = 1;
indexInfo->ii_IndexAttrNumbers[1] = 2;
indexInfo->ii_Expressions = NIL;
indexInfo->ii_ExpressionsState = NIL;
indexInfo->ii_Predicate = NIL;
indexInfo->ii_PredicateState = NULL;
indexInfo->ii_ExclusionOps = NULL;
indexInfo->ii_ExclusionProcs = NULL;
indexInfo->ii_ExclusionStrats = NULL;
indexInfo->ii_Unique = true;
indexInfo->ii_NullsNotDistinct = false;
indexInfo->ii_ReadyForInserts = true;
indexInfo->ii_CheckedUnchanged = false;
indexInfo->ii_IndexUnchanged = false;
indexInfo->ii_Concurrent = false;
indexInfo->ii_BrokenHotChain = false;
indexInfo->ii_ParallelWorkers = 0;
indexInfo->ii_Am = BTREE_AM_OID;
indexInfo->ii_AmCache = NULL;
indexInfo->ii_Context = CurrentMemoryContext;
```
There are more complicated cases. For instance in execMain.c (and
similarly elsewhere) we have:
```
rInfo = makeNode(ResultRelInfo);
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
rootRelInfo,
estate->es_instrument);
```
... where InitResultRelInfo does:
```
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo)); // !!!
resultRelInfo->type = T_ResultRelInfo;
resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
resultRelInfo->ri_RelationDesc = resultRelationDesc;
resultRelInfo->ri_NumIndices = 0;
resultRelInfo->ri_IndexRelationDescs = NULL;
resultRelInfo->ri_IndexRelationInfo = NULL;
resultRelInfo->ri_needLockTagTuple =
IsInplaceUpdateRelation(resultRelationDesc);
```
Here the fields are initialized 3 times in a row which is rather strange IMO.
In the same time other places e.g. copy.c may just do:
```
select = makeNode(SelectStmt);
select->targetList = targetList;
select->fromClause = list_make1(from);
```
IMO zeroing structures once is preferable in terms of both performance
and readability. Unless there are strong objections I would like to
propose a refactoring that would make the code consistent and use the
pattern as in the latest example, perhaps with few exceptions e.g. for
enum values that happened to be zero. Please let me know whether
`false` and perhaps other values should be an exception too.
If there are objections, I would like to know which method of
initialization is preferable so that alternatively we could follow
this pattern. Perhaps makeNode() shouldn't in fact use palloc0()? Or
maybe we should generate functions like makeSelectStmt() and
makeIndexInfo() ?
Honestly I don't know which approach is best and whether this is even
a problem we should invest our time into. Please let me know what you
think.
--
Best regards,
Aleksander Alekseev
