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