On Fri, Jun 26, 2020 at 6:15 PM Rushabh Lathia <rushabh.lat...@gmail.com> wrote: > > > > On Fri, Jun 26, 2020 at 3:16 PM Bharath Rupireddy > <bharath.rupireddyforpostg...@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
