From d51924be886c2a05e691fa05b16cb6b30ab8370f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 3 Oct 2016 22:27:11 -0400 Subject: [PATCH] Convert contrib/hstore_plpython to not use direct linking to other modules. Previously, on most platforms, we allowed hstore_plpython's references to hstore and plpython to be unresolved symbols at link time, trusting the dynamic linker to resolve them when the module is loaded. This has a number of problems, the worst being that the dynamic linker does not know where the references come from and can do nothing but fail if those other modules haven't been loaded. We've more or less gotten away with that for the limited use-case of datatype transform modules, but even there, it requires some awkward hacks, most recently commit 83c249200. Instead, let's not treat these references as linker-resolvable at all, but use function pointers that are manually filled in by the module's _PG_init function. There are few enough contact points that this doesn't seem unmaintainable, at least for these use-cases. (Note that the same technique wouldn't work at all for decoupling from libpython itself, but fortunately that's just a standard shared library and can be linked to normally.) This is an initial patch that just converts hstore_plpython. If the buildfarm doesn't find any fatal problems, I'll work on the other transform modules soon. Tom Lane, per an idea of Andres Freund's. Discussion: <2652.1475512158@sss.pgh.pa.us> --- contrib/hstore_plpython/Makefile | 19 +++--- contrib/hstore_plpython/hstore_plpython.c | 65 +++++++++++++++++++ .../hstore_plpython2u--1.0.sql | 5 -- .../hstore_plpython3u--1.0.sql | 5 -- .../hstore_plpython/hstore_plpythonu--1.0.sql | 5 -- src/tools/msvc/Mkvcbuild.pm | 3 +- 6 files changed, 76 insertions(+), 26 deletions(-) diff --git a/contrib/hstore_plpython/Makefile b/contrib/hstore_plpython/Makefile index c4dad6f111..a55c9a162c 100644 --- a/contrib/hstore_plpython/Makefile +++ b/contrib/hstore_plpython/Makefile @@ -4,7 +4,7 @@ MODULE_big = hstore_plpython$(python_majorversion) OBJS = hstore_plpython.o $(WIN32RES) PGFILEDESC = "hstore_plpython - hstore transform for plpython" -PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/hstore +PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/hstore -DPLPYTHON_LIBNAME='"plpython$(python_majorversion)"' EXTENSION = hstore_plpythonu hstore_plpython2u hstore_plpython3u DATA = hstore_plpythonu--1.0.sql hstore_plpython2u--1.0.sql hstore_plpython3u--1.0.sql @@ -23,19 +23,18 @@ include $(top_builddir)/src/Makefile.global include $(top_srcdir)/contrib/contrib-global.mk endif -# In configurations that forbid undefined symbols in libraries, link with each -# dependency. This does preclude pgxs builds. +# We must link libpython explicitly ifeq ($(PORTNAME), aix) rpathdir = $(pkglibdir):$(python_libdir) -SHLIB_LINK += ../hstore/libhstore.exp $(python_libspec) $(python_additional_libs) $(sort $(wildcard ../../src/pl/plpython/libplpython*.exp)) -endif +SHLIB_LINK += $(python_libspec) $(python_additional_libs) +else ifeq ($(PORTNAME), win32) -SHLIB_LINK += ../hstore/libhstore.a $(sort $(wildcard ../../src/pl/plpython/libpython*.a)) $(sort $(wildcard ../../src/pl/plpython/libplpython*.a)) +# ... see silliness in plpython Makefile ... +SHLIB_LINK += $(sort $(wildcard ../../src/pl/plpython/libpython*.a)) +else +rpathdir = $(python_libdir) +SHLIB_LINK += $(python_libspec) endif - -ifeq ($(PORTNAME), cygwin) -SHLIB_LINK += -L../hstore -lhstore -L../../src/pl/plpython \ - -lplpython$(python_majorversion) $(python_libspec) endif REGRESS_OPTS += --load-extension=hstore diff --git a/contrib/hstore_plpython/hstore_plpython.c b/contrib/hstore_plpython/hstore_plpython.c index 6f2751a8df..ce296ca486 100644 --- a/contrib/hstore_plpython/hstore_plpython.c +++ b/contrib/hstore_plpython/hstore_plpython.c @@ -1,4 +1,5 @@ #include "postgres.h" + #include "fmgr.h" #include "plpython.h" #include "plpy_typeio.h" @@ -6,6 +7,70 @@ PG_MODULE_MAGIC; +extern void _PG_init(void); + +/* Linkage to functions in plpython module */ +typedef char *(*PLyObject_AsString_t) (PyObject *plrv); + +static PLyObject_AsString_t PLyObject_AsString_p; + +/* Linkage to functions in hstore module */ +typedef HStore *(*hstoreUpgrade_t) (Datum orig); +typedef int (*hstoreUniquePairs_t) (Pairs *a, int32 l, int32 *buflen); +typedef HStore *(*hstorePairs_t) (Pairs *pairs, int32 pcount, int32 buflen); +typedef size_t (*hstoreCheckKeyLen_t) (size_t len); +typedef size_t (*hstoreCheckValLen_t) (size_t len); + +static hstoreUpgrade_t hstoreUpgrade_p; +static hstoreUniquePairs_t hstoreUniquePairs_p; +static hstorePairs_t hstorePairs_p; +static hstoreCheckKeyLen_t hstoreCheckKeyLen_p; +static hstoreCheckValLen_t hstoreCheckValLen_p; + + +/* + * Module initialize function: fetch function pointers for cross-module calls. + */ +void +_PG_init(void) +{ + /* These asserts verify that typedefs above match original declarations */ + AssertVariableIsOfType(&PLyObject_AsString, PLyObject_AsString_t); + AssertVariableIsOfType(&hstoreUpgrade, hstoreUpgrade_t); + AssertVariableIsOfType(&hstoreUniquePairs, hstoreUniquePairs_t); + AssertVariableIsOfType(&hstorePairs, hstorePairs_t); + AssertVariableIsOfType(&hstoreCheckKeyLen, hstoreCheckKeyLen_t); + AssertVariableIsOfType(&hstoreCheckValLen, hstoreCheckValLen_t); + + PLyObject_AsString_p = (PLyObject_AsString_t) + load_external_function("$libdir/" PLPYTHON_LIBNAME, "PLyObject_AsString", + true, NULL); + hstoreUpgrade_p = (hstoreUpgrade_t) + load_external_function("$libdir/hstore", "hstoreUpgrade", + true, NULL); + hstoreUniquePairs_p = (hstoreUniquePairs_t) + load_external_function("$libdir/hstore", "hstoreUniquePairs", + true, NULL); + hstorePairs_p = (hstorePairs_t) + load_external_function("$libdir/hstore", "hstorePairs", + true, NULL); + hstoreCheckKeyLen_p = (hstoreCheckKeyLen_t) + load_external_function("$libdir/hstore", "hstoreCheckKeyLen", + true, NULL); + hstoreCheckValLen_p = (hstoreCheckValLen_t) + load_external_function("$libdir/hstore", "hstoreCheckValLen", + true, NULL); +} + + +/* These defines must be after the module init function */ +#define PLyObject_AsString PLyObject_AsString_p +#define hstoreUpgrade hstoreUpgrade_p +#define hstoreUniquePairs hstoreUniquePairs_p +#define hstorePairs hstorePairs_p +#define hstoreCheckKeyLen hstoreCheckKeyLen_p +#define hstoreCheckValLen hstoreCheckValLen_p + PG_FUNCTION_INFO_V1(hstore_to_plpython); diff --git a/contrib/hstore_plpython/hstore_plpython2u--1.0.sql b/contrib/hstore_plpython/hstore_plpython2u--1.0.sql index e3aea6399e..800765f3f0 100644 --- a/contrib/hstore_plpython/hstore_plpython2u--1.0.sql +++ b/contrib/hstore_plpython/hstore_plpython2u--1.0.sql @@ -3,11 +3,6 @@ -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use "CREATE EXTENSION hstore_plpython2u" to load this file. \quit --- make sure the prerequisite libraries are loaded -LOAD 'plpython2'; -SELECT NULL::hstore; - - CREATE FUNCTION hstore_to_plpython2(val internal) RETURNS internal LANGUAGE C STRICT IMMUTABLE AS 'MODULE_PATHNAME', 'hstore_to_plpython'; diff --git a/contrib/hstore_plpython/hstore_plpython3u--1.0.sql b/contrib/hstore_plpython/hstore_plpython3u--1.0.sql index a964a49059..0b410ab183 100644 --- a/contrib/hstore_plpython/hstore_plpython3u--1.0.sql +++ b/contrib/hstore_plpython/hstore_plpython3u--1.0.sql @@ -3,11 +3,6 @@ -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use "CREATE EXTENSION hstore_plpython3u" to load this file. \quit --- make sure the prerequisite libraries are loaded -LOAD 'plpython3'; -SELECT NULL::hstore; - - CREATE FUNCTION hstore_to_plpython3(val internal) RETURNS internal LANGUAGE C STRICT IMMUTABLE AS 'MODULE_PATHNAME', 'hstore_to_plpython'; diff --git a/contrib/hstore_plpython/hstore_plpythonu--1.0.sql b/contrib/hstore_plpython/hstore_plpythonu--1.0.sql index d79bdc96d9..52832912ab 100644 --- a/contrib/hstore_plpython/hstore_plpythonu--1.0.sql +++ b/contrib/hstore_plpython/hstore_plpythonu--1.0.sql @@ -3,11 +3,6 @@ -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use "CREATE EXTENSION hstore_plpythonu" to load this file. \quit --- make sure the prerequisite libraries are loaded -LOAD 'plpython2'; -- change to plpython3 if that ever becomes the default -SELECT NULL::hstore; - - CREATE FUNCTION hstore_to_plpython(val internal) RETURNS internal LANGUAGE C STRICT IMMUTABLE AS 'MODULE_PATHNAME'; diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 5add0b8a91..d2ab9e466e 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -475,10 +475,11 @@ sub mkvcbuild $plpython->AddReference($postgres); # Add transform modules dependent on plpython - AddTransformModule( + my $hstore_plpython = AddTransformModule( 'hstore_plpython' . $pymajorver, 'contrib/hstore_plpython', 'plpython' . $pymajorver, 'src/pl/plpython', 'hstore', 'contrib/hstore'); + $hstore_plpython->AddDefine('PLPYTHON_LIBNAME="plpython' . $pymajorver . '"'); AddTransformModule( 'ltree_plpython' . $pymajorver, 'contrib/ltree_plpython', 'plpython' . $pymajorver, 'src/pl/plpython', -- 2.40.0