From: | Anthony Bykov <a(dot)bykov(at)postgrespro(dot)ru> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Transform for pl/perl |
Date: | 2018-03-12 08:08:21 |
Message-ID: | 20180312110821.29c80420@anthony-24-g082ur |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 5 Mar 2018 14:03:37 +0100
Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hi
>
> I am looking on this patch. I found few issues:
>
> 1. compile warning
>
> I../../src/include -D_GNU_SOURCE -I/usr/include/libxml2
> -I/usr/lib64/perl5/CORE -c -o jsonb_plperl.o jsonb_plperl.c
> jsonb_plperl.c: In function ‘SV_FromJsonbValue’:
> jsonb_plperl.c:69:9: warning: ‘result’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> return result;
> ^~~~~~
> jsonb_plperl.c: In function ‘SV_FromJsonb’:
> jsonb_plperl.c:142:9: warning: ‘result’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> return result;
> ^~~~~~
>
> 2. bad comment
>
> /*
> * SV_ToJsonbValue
> *
> * Transform Jsonb into SV --- propably reverse direction
> */
>
>
> /*
> * HV_ToJsonbValue
> *
> * Transform Jsonb into SV
> */
>
> /*
> * plperl_to_jsonb(SV *in)
> *
> * Transform Jsonb into SV
> */
>
> 3. Do we need two identical tests fro PLPerl and PLPerlu? Why?
>
> Regards
>
> Pavel
Hello, thanks for reviewing my patch! I really appreciate it.
That warnings are created on purpose - I was trying to prevent the case
when new types are added into pl/perl, but new transform logic was not.
Maybe there is a better way to do it, but right now I'll just add the
"default: pg_unreachable" logic.
About the 3 point - I thought that plperlu and plperl uses different
interpreters. And if they act identically on same examples - there
is no need in identical tests for them indeed.
Point 2 is fixed in this version of the patch.
Please, find in attachments a new version of the patch.
--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
0001-jsonb_plperl-extension-v8.patch | text/x-patch | 32.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Aleksander Alekseev | 2018-03-12 08:25:03 | Re: GSOC 2018 proposal |
Previous Message | Jeevan Ladhe | 2018-03-12 08:07:21 | Fix error in ECPG while connection handling |