Re: Narrow the scope of the variable outputstr in logicalrep_write_tuple

From: japin <japinli(at)hotmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Narrow the scope of the variable outputstr in logicalrep_write_tuple
Date: 2021-01-18 08:09:17
Message-ID: MEYP282MB16697AA10DBD5EDAA5085CBFB6A40@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Mon, 18 Jan 2021 at 15:59, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> On Mon, Jan 18, 2021 at 1:16 PM japin <japinli(at)hotmail(dot)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.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachment Content-Type Size
v2-0001-Narrow-the-scope-of-the-variable-outputstr-in-log.patch text/x-patch 1.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2021-01-18 08:17:05 Re: Code of Conduct plan,Re: Code of Conduct plan
Previous Message Bharath Rupireddy 2021-01-18 07:59:51 Re: Narrow the scope of the variable outputstr in logicalrep_write_tuple