From: Aleksander Alekseev <a.alekseev@postgrespro.ru>
To: pgsql-hackers@postgresql.org
Cc: Anthony Bykov <a.bykov@postgrespro.ru>
Subject: Re: Jsonb transform for pl/python
Date: Thu, 09 Nov 2017 12:26:46 +0000
Message-ID: <20171109122646.1353.77959.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <20171025145100.3a19933f@anthony-24-g082ur>
References: <20171025145100.3a19933f@anthony-24-g082ur>
<20171025145100.3a19933f@anthony-24-g082ur>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
Hello Anthony,
Great job!
I decided to take a closer look on your patch. Here are some defects I
discovered.
> + Additional extensions are available that implement transforms for
> + the <type>jsonb</type> type for the language PL/Python. The
> + extensions for PL/Perl are called
1. The part regarding PL/Perl is obviously from another patch.
2. jsonb_plpython2u and jsonb_plpythonu are marked as relocatable, while
jsonb_plpython3u is not. Is it a mistake? Anyway if an extension is relocatable
there should be a test that checks this.
3. Not all json types are test-covered. Tests for 'true' :: jsonb, '3.14' ::
jsonb and 'null' :: jsonb are missing.
4. jsonb_plpython.c:133 - "Iterate trhrough Jsonb object." Typo, it should be
"through" or probably even "over".
5. It looks like you've implemented transform in two directions Python <->
JSONB, however I see tests only for Python <- JSONB case.
6. Tests passed on Python 2.7.14 but failed on 3.6.2:
> CREATE EXTENSION jsonb_plpython3u CASCADE;
> + ERROR: could not access file "$libdir/jsonb_plpython3": No such file or
> directory
module_pathname in jsonb_plpython3u.control should be $libdir/jsonb_plpython3u,
not $libdir/jsonb_plpython3.
Tested on Arch Linux x64, GCC 7.2.0.
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.alekseev@postgrespro.ru, a.bykov@postgrespro.ru
Subject: Re: Jsonb transform for pl/python
In-Reply-To: <20171109122646.1353.77959.pgcf@coridan.postgresql.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox