Re: Transform for pl/perl
От | Anthony Bykov |
---|---|
Тема | Re: Transform for pl/perl |
Дата | |
Msg-id | 20180312110821.29c80420@anthony-24-g082ur обсуждение исходный текст |
Ответ на | Re: Transform for pl/perl (Pavel Stehule <pavel.stehule@gmail.com>) |
Ответы |
Re: Transform for pl/perl
|
Список | pgsql-hackers |
On Mon, 5 Mar 2018 14:03:37 +0100 Pavel Stehule <pavel.stehule@gmail.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
Вложения
В списке pgsql-hackers по дате отправления: