Re: Narrow the scope of the variable outputstr in logicalrep_write_tuple

On Mon, Jan 18, 2021 at 1:39 PM japin <japinli@hotmail.com> wrote:
>
>
> On Mon, 18 Jan 2021 at 15:59, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
> > On Mon, Jan 18, 2021 at 1:16 PM japin <japinli@hotmail.com> wrote:
> >>
> >>
> >> Hi,
> >>
> >> I find that the outputstr variable in logicalrep_write_tuple() only use in
> >> `else` branch, I think we can narrow the scope, just like variable outputbytes
> >> in `if` branch (for more readable).
> >>
> >>         /*
> >>          * Send in binary if requested and type has suitable send function.
> >>          */
> >>         if (binary && OidIsValid(typclass->typsend))
> >>         {
> >>             bytea      *outputbytes;
> >>             int         len;
> >>
> >>             pq_sendbyte(out, LOGICALREP_COLUMN_BINARY);
> >>             outputbytes = OidSendFunctionCall(typclass->typsend, values[i]);
> >>             len = VARSIZE(outputbytes) - VARHDRSZ;
> >>             pq_sendint(out, len, 4);    /* length */
> >>             pq_sendbytes(out, VARDATA(outputbytes), len);   /* data */
> >>             pfree(outputbytes);
> >>         }
> >>         else
> >>         {
> >>             pq_sendbyte(out, LOGICALREP_COLUMN_TEXT);
> >>             outputstr = OidOutputFunctionCall(typclass->typoutput, values[i]);
> >>             pq_sendcountedtext(out, outputstr, strlen(outputstr), false);
> >>             pfree(outputstr);
> >>         }
> >>
> >> Attached is a samll patch to fix it.
> >
> > +1. Binary mode uses block level variable outputbytes, so making
> > outputstr block level is fine IMO.
> >
> > Patch basically looks good to me, but it doesn't apply on my system.
> > Looks like it's not created with git commit. Please create the patch
> > with git commit command.
> >
> > git apply /mnt/hgfs/Shared/narrow-the-scope-of-the-variable-in-logicalrep_write_tuple.patch
> > error: corrupt patch at line 10
> >
>
> Thanks for reviewing! Attached v2 as you suggested.

Thanks. v2 patch LGTM.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com