From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Anthony Bykov <a(dot)bykov(at)postgrespro(dot)ru> |
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 15:06:53 |
Message-ID: | CAFj8pRDaTYHD30CP8u2v_wwsJfxFBfthcNEp_KgEOHVN0gUfAw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2018-03-12 9:08 GMT+01:00 Anthony Bykov <a(dot)bykov(at)postgrespro(dot)ru>:
> 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.
>
plperlu and plperl uses same interprets - so the duplicate tests has not
any sense. But in last versions there are duplicate tests again
The naming convention of functions is not consistent
almost are are src_to_dest
This is different and it is little bit messy
+static SV *
+SV_FromJsonb(JsonbContainer *jsonb)
This comment is broken
+/*
+ * plperl_to_jsonb(SV *in)
+ *
+ * Transform Jsonb into SV ---< should be SV to Jsonb
+ */
+PG_FUNCTION_INFO_V1(plperl_to_jsonb);
+Datum
Regards
Pavel
>
> 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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2018-03-12 15:21:36 | Re: JIT compiling with LLVM v11 |
Previous Message | Tom Lane | 2018-03-12 15:00:19 | Re: Cast jsonb to numeric, int, float, bool |