On Mon, 18 Jan 2021 at 15:59, Bharath Rupireddy 
<bharath.rupireddyforpostg...@gmail.com> wrote:
> On Mon, Jan 18, 2021 at 1:16 PM japin <japi...@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.

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

>From 89fb10efc9eccf11f531ab8675376b57a12a6cb1 Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Mon, 18 Jan 2021 16:04:54 +0800
Subject: [PATCH v2] Narrow the scope of the variable outputstr in
 logicalrep_write_tuple

---
 src/backend/replication/logical/proto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index 62275ebabe..f2c85cabb5 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -493,7 +493,6 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple, bool binar
 		HeapTuple	typtup;
 		Form_pg_type typclass;
 		Form_pg_attribute att = TupleDescAttr(desc, i);
-		char	   *outputstr;
 
 		if (att->attisdropped || att->attgenerated)
 			continue;
@@ -537,6 +536,8 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple, bool binar
 		}
 		else
 		{
+			char	   *outputstr;
+
 			pq_sendbyte(out, LOGICALREP_COLUMN_TEXT);
 			outputstr = OidOutputFunctionCall(typclass->typoutput, values[i]);
 			pq_sendcountedtext(out, outputstr, strlen(outputstr), false);
-- 
2.30.0

Reply via email to