Re: [HACKERS] Jsonb transform for pl/python

2017-11-13 Thread Anthony Bykov
On Thu, 09 Nov 2017 12:26:46 +
Aleksander Alekseev  wrote:

> 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 jsonb 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
> 

Hello, Aleksander. 
Thank you for your time. The defects you have noticed were fixed.
Please, find in attachments new version of the patch (it is called
0001-jsonb_plpython-extension-v2.patch).

Most of changes were made to fix defects(list of the defects may be
found in citation in the beginning of this message), but the algorithm
of iterating through incoming jsonb was changed so that it looks tidier.

--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/contrib/Makefile b/contrib/Makefile
index 8046ca4..d9d9817 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -81,9 +81,9 @@ ALWAYS_SUBDIRS += hstore_plperl
 endif
 
 ifeq ($(with_python),yes)
-SUBDIRS += hstore_plpython ltree_plpython
+SUBDIRS += hstore_plpython ltree_plpython jsonb_plpython
 else
-ALWAYS_SUBDIRS += hstore_plpython ltree_plpython
+ALWAYS_SUBDIRS += hstore_plpython ltree_plpython jsonb_plpython
 endif
 
 # Missing:
diff --git a/contrib/jsonb_plpython/Makefile b/contrib/jsonb_plpython/Makefile
new file mode 100644
index 000..6371d11
--- /dev/null
+++ b/contrib/jsonb_plpython/Makefile
@@ -0,0 +1,39 @@
+# contrib/jsonb_plpython/Makefile
+
+MODULE_big = jsonb_plpython$(python_majorversion)u
+OBJS = jsonb_plpython.o $(WIN32RES)
+PGFILEDESC = "jsonb_plpython - transform between jsonb and plpythonu"
+
+PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -DPLPYTHON_LIBNAME='"plpython$(python_majorversion)"'
+
+EXTENSION = jsonb_plpythonu jsonb_plpython2u jsonb_plpython3u
+DATA = jsonb_plpythonu--1.0.sql jsonb_plpython2u--1.0.sql jsonb_plpython3u--1.0.sql
+
+REGRESS = jsonb_plpython$(python_majorversion)
+REGRESS_PLPYTHON3_MANGLE := $(REGRESS)
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/jsonb_plpython
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+# We must link libpython explicitly
+ifeq ($(PORTNAME), win32)
+# ... see silliness in plpython Makefile ...
+SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a))
+else
+rpathdir = $(python_libdir)
+SHLIB_LINK += $(python_libspec) $(python_additional_libs)
+endif
+
+ifeq ($(python_majorversion),2)
+REGRESS_OPTS += --load-extension=plpython2u
+else
+REGRESS_OPTS += --load-extension=plpython3u
+endif
diff --git a/contrib/jsonb_plpython/expected/jsonb_plpython2.out b/contrib/jsonb_plpython/expected/jsonb_plpython2.out
new file mode 100644
index 000..8ad5338
--- /dev/null
+++ b/contrib/jsonb_plpython/expected/jsonb_plpython2.out
@@ -0,0 +1,478 @@
+CREATE EXTENSION jsonb_plpython2u CASCADE;
+-- test jsonb -> python dict
+CREATE FUNCTION test1(val jsonb) RETURNS int
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+assert isinstance(val, dict)
+plpy.info(sorted(val.items()))
+return len(val)
+$$;
+SELECT test1('{"a":1, "c":"NULL"}'::jsonb);
+INFO:  [('a', Decimal('1')), ('c', 'NULL')]
+ test1 
+---
+ 2
+(1 row)
+
+-- test jsonb -> python dict
+-- complex dict with dicts as value
+CREATE FUNCTION test1complex(val jsonb) RETURNS int
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+assert isinstance(val, dict)
+assert(val == {"d":{"d": 1}})
+return len(val)
+$$;
+SELECT test1comple

Re: [HACKERS] Jsonb transform for pl/python

2017-11-09 Thread Aleksander Alekseev
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 jsonb 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


Re: [HACKERS] Jsonb transform for pl/python

2017-10-30 Thread David Fetter
On Mon, Oct 30, 2017 at 11:15:00AM +0300, Anthony Bykov wrote:
> On Sun, 29 Oct 2017 19:11:02 +0100
> David Fetter  wrote:
> 
> > Thanks for your hard work!
> > 
> > Should there also be one for PL/Python3U?
> > 
> > Best,
> > David.
> Hi.
> Actually, there is one for PL/Python3U. This patch contains following
> extensions:
> jsonb_plpythonu
> jsonb_plpython2u
> jsonb_plpython3u
> "make install" checks which python major version was your postgresql
> configured with and installs corresponding extension.

My mistake.  Sorry about the noise.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Jsonb transform for pl/python

2017-10-30 Thread Anthony Bykov
On Sun, 29 Oct 2017 19:11:02 +0100
David Fetter  wrote:

> Thanks for your hard work!
> 
> Should there also be one for PL/Python3U?
> 
> Best,
> David.
Hi.
Actually, there is one for PL/Python3U. This patch contains following
extensions:
jsonb_plpythonu
jsonb_plpython2u
jsonb_plpython3u
"make install" checks which python major version was your postgresql
configured with and installs corresponding extension.

--
Anthony Bykov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Jsonb transform for pl/python

2017-10-29 Thread David Fetter
On Wed, Oct 25, 2017 at 02:51:00PM +0300, Anthony Bykov wrote:
> Hi.
> I've implemented jsonb transform
> (https://www.postgresql.org/docs/9.5/static/sql-createtransform.html)
> for pl/python. 
> 
> 1. '{"1":1}'::jsonb is transformed into dict {"1"=>1}, while
> '["1",2]'::jsonb is transformed into list(not tuple!) ["1", 2]
> 
> 2. If there is a numeric value appear in jsonb, it will be transformed
> to decimal through string (Numeric->String->Decimal). Not the best
> solution, but as far as I understand this is usual practise in
> postgresql to serialize Numerics and de-serialize them.
> 
> 3. Decimal is transformed into jsonb through string
> (Decimal->String->Numeric).
> 
> An example may also be helpful to understand extension. So, as an
> example, function "test" transforms incoming jsonb into python,
> transforms it back into jsonb and returns it.
> 
> create extension jsonb_plpython2u cascade;

Thanks for your hard work!

Should there also be one for PL/Python3U?

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers