Re: Jsonb transform for pl/python

From: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Anthony Bykov <a(dot)bykov(at)postgrespro(dot)ru>
Subject: Re: Jsonb transform for pl/python
Date: 2017-11-09 12:26:46
Message-ID: 20171109122646.1353.77959.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Arthur Zakirov 2017-11-09 12:29:40 Re: pg_basebackup --progress output for batch execution
Previous Message Graham Leggett 2017-11-09 11:27:58 libpq connection strings: control over the cipher suites?