Hi Vignesh,
After having a look over the patch,
I have some suggestions for
0003-Allow-copy-from-command-to-process-data-from-file.patch.
1.
+static uint32
+EstimateCstateSize(ParallelContext *pcxt, CopyState cstate, List *attnamelist,
+ char **whereClauseStr, char **rangeTableStr,
+ char **attnameListStr, char **notnullListStr,
+ char **nullListStr, char **convertListStr)
+{
+ uint32 strsize = MAXALIGN(sizeof(SerializedParallelCopyState));
+
+ strsize += EstimateStringSize(cstate->null_print);
+ strsize += EstimateStringSize(cstate->delim);
+ strsize += EstimateStringSize(cstate->quote);
+ strsize += EstimateStringSize(cstate->escape);
It use function EstimateStringSize to get the strlen of null_print, delim, quote and escape.
But the length of null_print seems has been stored in null_print_len.
And delim/quote/escape must be 1 byte, so I think call strlen again seems unnecessary.
How about " strsize += sizeof(uint32) + cstate->null_print_len + 1"
2.
+ strsize += EstimateNodeSize(cstate->whereClause, whereClauseStr);
+ copiedsize += CopyStringToSharedMemory(cstate, whereClauseStr,
+ shmptr + copiedsize);
Some string length is counted for two times.
The ' whereClauseStr ' has call strlen in EstimateNodeSize once and call strlen in CopyStringToSharedMemory again.
I don't know wheather it's worth to refacor the code to avoid duplicate strlen . what do you think ?
Best regards,
houzj
