Re: [PATCH] Remove Extra palloc Of raw_buf For Binary Format In COPY FROM

On Fri, Jun 26, 2020 at 6:15 PM Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
>
>
>
> On Fri, Jun 26, 2020 at 3:16 PM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
>>
>> Hi Hackers,
>>
>> There seems to be an extra palloc of 64KB of raw_buf for binary format
>> files which is not required
>> as copy logic for binary files don't use raw_buf, instead, attribute_buf
>> is used in CopyReadBinaryAttribute.
>
>
> +1
>
> I looked at the patch and the changes looked good. Couple of comments;
>
> 1)
>
> +
> + /* For binary files raw_buf is not used,
> + * instead, attribute_buf is used in
> + * CopyReadBinaryAttribute. Hence, don't palloc
> + * raw_buf.
> + */
>
> Not a PG style of commenting.
>
> 2)  In non-binary mode, should assign NULL the raw_buf.
>
> Attaching patch with those changes.
>

+1 for the patch.

One comment:
We could change below code:
+ */
+ if (!cstate->binary)
+ cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
+ else
+ cstate->raw_buf = NULL;
to:
cstate->raw_buf = (cstate->binary) ? NULL : (char *) palloc(RAW_BUF_SIZE + 1);

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com