transforms
transforms
Here is my first patch for the transforms feature. This is a mechanism
to adapt data types to procedural languages. The previous proposal was
here: http://archives.postgresql.org/pgsql-hackers/2012-05/msg00728.php
At the surface, this contains:
- New catalog pg_transform
- CREATE/DROP TRANSFORM
As proof of concepts and hopefully useful applications to-be, I have
included transforms for
- PL/Perl - hstore
- PL/Python - hstore
- PL/Python - ltree
These, along with the documentation for the above-mentioned DDL commands
and catalog can serve as explanations for reviewers.
There are numerous remaining discussion points and possible work items:
*) Currently, each of the above provided transforms is put into a
separate subdirectory under contrib. This might be OK, but it is mainly
because we cannot build more than one MODULE_big per directory. Fixing
that might be more work than it's worth, but if we want to provide more
of these in contrib, it might get crowded.
*) Since we have separate extensions for plperl and plperlu, and also
for plpython2u and plpython3u, we need one extension for adapting each
of these to a given type. You can see under contrib/hstore_plperl what
this looks like. Maybe this isn't fixable or worth fixing.
(With the directory and packaging not finalized, I haven't included any
documentation for these contrib modules yet.)
*) No psql backslash commands yet. Could be added.
*) Permissions: Transforms don't have owners, a bit like casts.
Currently, you are allowed to drop a transform if you own both the type
and the language. That might be too strict, maybe own the type and have
privileges on the language would be enough.
*) If we want to offer the ability to write transforms to end users,
then we need to install all the header files for the languages and the
types. This actually worked quite well; including hstore.h and plperl.h
for example, gave you what you needed. In other cases, some headers
might need cleaning up. Also, some magic from the plperl and plpython
build systems might need to be exposed, for example to find the header
files. See existing modules for how this currently looks.
*) There is currently some syntax schizophrenia. The grammar accepts
CREATE TRANSFORM FOR hstore LANGUAGE plperl (
FROM SQL WITH hstore_to_plperl,
TO SQL WITH plperl_to_hstore
);
but pg_dump produces
CREATE TRANSFORM FOR hstore LANGUAGE plperl (
FROM SQL WITH hstore_to_plperl(hstore),
TO SQL WITH plperl_to_hstore(internal)
);
The SQL standard allows both. (In the same way that it allows 'DROP
FUNCTION foo' without arguments, if it is not ambigious.) Precedent is
that CREATE CAST requires arguments, but CREATE LANGUAGE does not.
*) The issue of how to handle arguments of type "internal" was already
discussed at
http://archives.postgresql.org/pgsql-hackers/2012-05/msg00857.php and
following. I have adopted the suggestion there to prohibit calling
functions with arguments of type "internal", but that is probably full
of holes and needs to be reviewed carefully.
*) I'm not very familiar with the Perl C API, so the hstore_plperl
implementation is probably full of memory leaks and weirdnesses. It
needs some help from a PL/Perl expert. (The PL/Python stuff is
hopefully better.)
*) ltree_plpython lacks the transform function from python to ltree,
because of laziness and lack of clear documentation on how to construct
ltree values.
*) I just noticed that the grammar changes broke ECPG. I'll look into
that.
*) The regression tests for the core CREATE/DROP TRANSFORM functionality
is in contrib/hstore_plpython, because the core has no suitable language
available. It's a bit ugly, but I don't know of a better way short of
implementing a fake language for regression testing only.
Re: transforms
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Thu, Jun 14, 2012 at 3:42 PM, Peter Eisentraut wrote: > Here is my first patch for the transforms feature. This is a mechanism > to adapt data types to procedural languages. The previous proposal was > here: http://archives.postgresql.org/pgsql-hackers/2012-05/msg00728.php When I apply this and go to contrib and do make check, I get: In file included from hstore_plperl.c:4:0: ../../src/pl/plperl/plperl.h:49:20: fatal error: EXTERN.h: No such file or directory Cheers, Jeff
Re: transforms
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Sat, Jun 16, 2012 at 7:15 PM, Jeff Janes wrote: > On Thu, Jun 14, 2012 at 3:42 PM, Peter Eisentraut wrote: >> Here is my first patch for the transforms feature. This is a mechanism >> to adapt data types to procedural languages. The previous proposal was >> here: http://archives.postgresql.org/pgsql-hackers/2012-05/msg00728.php > > When I apply this and go to contrib and do make check, I get: > > In file included from hstore_plperl.c:4:0: > ../../src/pl/plperl/plperl.h:49:20: fatal error: EXTERN.h: No such > file or directory Ah, that went away when I remembered to ./configure --with-perl Although the error message seem less than optimal. Aren't test usually "skipped" when they are missing prerequisites in the config? Cheers, Jeff
Re: transforms
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
Hi Peter, On Friday, June 15, 2012 12:42:12 AM Peter Eisentraut wrote: > Here is my first patch for the transforms feature. This is a mechanism > to adapt data types to procedural languages. The previous proposal was > here: http://archives.postgresql.org/pgsql-hackers/2012-05/msg00728.php > > At the surface, this contains: > > - New catalog pg_transform > > - CREATE/DROP TRANSFORM Cursory code review: * pstrndup already exists as pnstrdup (hstore_plperl.c) * PyString_FromStringAndSize return value not decreffed? PyDict_SetItem doesn't steal references * In plpython_to_hstore I would move the 'pairs' and some related variables in the PG_TRY block, so the reader doesn't need to check whether it should be volatile * In the same function items needs to be volatile to fit into longjmp semantics * I don't think recording dependencies on transforms used when creating functions is a good idea as the transform might get created after the functions already exists. That seems to be a pretty confusing behaviour. * I am a bit wary that some place can be used to call functions accepting INTERNAL indirectly, couldn't think of any immediately though. Will look into this a bit, but I am not experienced in the area, so ... * I forsee the need for multiple transforms for the same type/language pair to coexist. The user would need to manually "choose"/"call" the transform in that case. This currently isn't easily possible... > *) No psql backslash commands yet. Could be added. Doesn't really seem necessary to me. Not many people will need to look at this and the list of commands already is rather long. > *) Permissions: Transforms don't have owners, a bit like casts. > Currently, you are allowed to drop a transform if you own both the type > and the language. That might be too strict, maybe own the type and have > privileges on the language would be enough. Seems sensible enough to me. > *) If we want to offer the ability to write transforms to end users, > then we need to install all the header files for the languages and the > types. This actually worked quite well; including hstore.h and plperl.h > for example, gave you what you needed. In other cases, some headers > might need cleaning up. Also, some magic from the plperl and plpython > build systems might need to be exposed, for example to find the header > files. See existing modules for how this currently looks. Doesn't look to bad to me. I dont't know how this could be made easier. > *) There is currently some syntax schizophrenia. The grammar accepts > > CREATE TRANSFORM FOR hstore LANGUAGE plperl ( > FROM SQL WITH hstore_to_plperl, > TO SQL WITH plperl_to_hstore > ); > > but pg_dump produces > > CREATE TRANSFORM FOR hstore LANGUAGE plperl ( > FROM SQL WITH hstore_to_plperl(hstore), > TO SQL WITH plperl_to_hstore(internal) > ); > > The SQL standard allows both. (In the same way that it allows 'DROP > FUNCTION foo' without arguments, if it is not ambigious.) Precedent is > that CREATE CAST requires arguments, but CREATE LANGUAGE does not. I don't find that problematic personally. Greetings, Andres -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: transforms
I haven't had the time to finish all the issues with this, but I want to keep the discussion going in the meantime and provide an updated patch. On mån, 2012-06-18 at 17:33 +0200, Andres Freund wrote: > Cursory code review: > * pstrndup already exists as pnstrdup (hstore_plperl.c) Fixed. > * PyString_FromStringAndSize return value not decreffed? PyDict_SetItem > doesn't steal references Fixed. > * In plpython_to_hstore I would move the 'pairs' and some related variables in > the PG_TRY block, so the reader doesn't need to check whether it should be > volatile > * In the same function items needs to be volatile to fit into longjmp > semantics I'll recheck that later. > * I don't think recording dependencies on transforms used when creating > functions is a good idea as the transform might get created after the > functions already exists. That seems to be a pretty confusing behaviour. We need the dependencies, because otherwise dropping a transform would break or silently alter the behavior of functions that depend on it. That sounds like my worst nightmare, thinking of some applications that would be affected by that. But your point is a good one. I think this could be addressed by prohibiting the creation of a transform that affects functions that already exist. Because the legacy behavior of PL implementations of defaulting to a string representation conversion, we would technically need a dependency on the absence of a transform object to make this airtight. In the far future, I could imagine removing this default behavior, meaning you couldn't create the function if no suitable transforms exist for all argument and return types. > * I forsee the need for multiple transforms for the same type/language pair to > coexist. The user would need to manually "choose"/"call" the transform in that > case. This currently isn't easily possible... I thought about this briefly at the beginning, but see under "worst nightmare" above. Also, having a configuration setting for this or something would prevent any PL functions from being immutable. We don't allow multiple casts or multiple in/out functions either, which are related concepts. If you want different behavior, you should define a different type or different language. > > *) No psql backslash commands yet. Could be added. > Doesn't really seem necessary to me. Not many people will need to look at this > and the list of commands already is rather long. I'm going to leave this out for now. > > *) Permissions: Transforms don't have owners, a bit like casts. > > Currently, you are allowed to drop a transform if you own both the type > > and the language. That might be too strict, maybe own the type and have > > privileges on the language would be enough. > Seems sensible enough to me. I have made this change. > > *) There is currently some syntax schizophrenia. The grammar accepts > > > > CREATE TRANSFORM FOR hstore LANGUAGE plperl ( > > FROM SQL WITH hstore_to_plperl, > > TO SQL WITH plperl_to_hstore > > ); > > > > but pg_dump produces > > > > CREATE TRANSFORM FOR hstore LANGUAGE plperl ( > > FROM SQL WITH hstore_to_plperl(hstore), > > TO SQL WITH plperl_to_hstore(internal) > > ); > > > > The SQL standard allows both. (In the same way that it allows 'DROP > > FUNCTION foo' without arguments, if it is not ambigious.) Precedent is > > that CREATE CAST requires arguments, but CREATE LANGUAGE does not. > I don't find that problematic personally. I have fixed the syntax to include argument types, so the dump output and the input grammar is consistent. Other changes: - Fixed ecpg grammar to work again with this. - Changed extension naming to be more consistent. - Build additional contrib modules conditionally depending on whether --with-perl or --with-python were configured. (complaint from Jeff Janes) - Fixed Python 3. Things I still want to do: - Refactor the regression test framework for Python 3 so that contrib modules or external extensions don't have to repeat the magic in src/pl/plpython/Makefile. (Python 3 with hstore_plpython and ltree_plpython works, but the tests don't run.) - Refactor pyobject_to_string(), which is currently kind of copied and pasted from plpython, but should instead be exported by plpython in some suitable way. - Refactor shared library building so that I can have, say, hstore, hstore_plperl, and hstore_plpython in one directory, rather than in three. The reason being, if someone has a new type in a repository on github or something, I don't want them to have to make three separate projects or some crazy subdirectory structure in order to add some PL support for their type. This will require some deep Makefile.shlib hacking, but I think it's worth trying to make this simple(r). So, it's quite likely that this patch won't get finished in this commit fest.
Re: transforms
Here is an updated patch for the transforms feature. The previous discussion was here: http://www.postgresql.org/message-id/1339713732.11971.79.camel@vanquo.pezone.net Old news: At the surface, this contains: - New catalog pg_transform - CREATE/DROP TRANSFORM As proofs of concept and useful applications, I have included transforms for - PL/Perl - hstore - PL/Python - hstore - PL/Python - ltree New news: I have tried to address all issues raised during previous reviews. The regression tests for hstore_plpython and ltree_plpython don't pass with Python 3. This needs to be fixed, obviously, but it's an issue unrelated to the core functionality. As a demo that this can be used externally as well, I have written a proof-of-concept transform between plpython and the mpz type in the pgmp extension (multi-precision arithmetic extension): https://github.com/petere/pgmp/tree/transforms/plpython Note: There was a lot of churn lately in src/backend/commands/, and I had to fix merge conflicts several times, so you need source as of about right now to apply this patch cleanly. Side issue/peculiarity: I had to change some shared library linking flags for OS X. Ordinarily, when linking a dynamically loadable module on OS X, if there are unresolved symbols, it's an error. But for example, when we link hstore_plpython, we expect some symbols to be resolved in hstore.so or plpythonu.so, so we need to ignore unresolved symbols. This change basically just makes OS X behave like other platforms in this regard. There might be other portability issues on other platforms.
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, I tried this patch out. I didn't get as far as testing the functionality because of errors. configure/make/make install/make check worked, without asserts. I believe DF found some errors when he enabled assertions. When I tried to install the actual transform extensions, though, it blew up with symbol errors: transforms=# create extension hstore transforms-# ; CREATE EXTENSION transforms=# create extension ltree transforms-# ; CREATE EXTENSION transforms=# create extension plperl transforms-# ; CREATE EXTENSION transforms=# create extension plpythonu transforms-# ; CREATE EXTENSION transforms=# create extension ltree_plpythonu; ERROR: could not load library "/home/josh/pg93/lib/postgresql/ltree_plpython2.so": /home/josh/pg93/lib/postgresql/ltree_plpython2.so: undefined symbol: PyList_New STATEMENT: create extension ltree_plpythonu; transforms=# create extension hstore_plperl; ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: hstoreUniquePairs STATEMENT: create extension hstore_plperl; This surprised me, because "make check" for the extensions passed fine. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, all: So in addition to the bugs I encountered in getting this patch to work, we have a design issue to work out: how to load all of the transform functions. Each transform depends on an optional datatype (like hstore) and an optional external language (like plperl), which can be loaded into the database in any order. Currently Peter is punting (as is proper in a new patch) by having a separate extension for each combination (hstore/plperl, hstore/plpython, ltree/plpython, etc.). This is obviously not a maintainable approach in the long run. In an ideal world, transformations would load automatically as soon as all of their prerequisites load. For example, if you load hstore and plpythonU, then hstore_plpython should automatically load whenever the second extension is loaded. However, we currently have no mechanism for this, especially since we don't know what order the user will load the two extensions in. Realistically, this would require enhancing the extension mechanism to track transformations. Discuss? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> transforms=# create extension hstore_plperl; > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: > hstoreUniquePairs > STATEMENT: create extension hstore_plperl; > > This surprised me, because "make check" for the extensions passed fine. Oh, this is on Ubuntu 12.10, not OSX. So possibly the fixes you made to fix linking on OSX broke other platforms. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Steve Singer <steve@ssinger.info>
Дата:
On 13-03-03 06:15 PM, Josh Berkus wrote: >> transforms=# create extension hstore_plperl; >> ERROR: could not load library >> "/home/josh/pg93/lib/postgresql/hstore_plperl.so": >> /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: >> hstoreUniquePairs >> STATEMENT: create extension hstore_plperl; >> >> This surprised me, because "make check" for the extensions passed fine. > Oh, this is on Ubuntu 12.10, not OSX. So possibly the fixes you made > to fix linking on OSX broke other platforms. > > This (creating the extensions) works fine for me on a Ubuntu 10.x system template1=# create database test; CREATE DATABASE template1=# \c test You are now connected to database "test" as user "ssinger". test=# create extension hstore; CREATE EXTENSION test=# create extension hstore_plpythonu; ERROR: required extension "plpythonu" is not installed STATEMENT: create extension hstore_plpythonu; ERROR: required extension "plpythonu" is not installed test=# create extension plpythonu; CREATE EXTENSION test=# create extension hstore_plpythonu; CREATE EXTENSION test=# test=# create extension plperl; CREATE EXTENSION test=# create extension hstore_plperl; CREATE EXTENSION test=# create extension plperlu; CREATE EXTENSION test=# create extension hstore_plperlu; CREATE EXTENSION test=#
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> This (creating the extensions) works fine for me on a Ubuntu 10.x system
>
Hmmmm. So I wiped everything out and retried this with clean
directories, and it worked fine. Now I'm not sure how I got the error
in the first place.
Anyway, the feature seems to work as expected:
create function look_up_hstore ( some_hs hstore, which_key text )
returns text
language plpythonu as $f$
return some_hs[which_key]
$f$;
postgres=# create table storit ( it hstore );
CREATE TABLE
postgres=# insert into storit values ( 'f => josh,l=>berkus'
),('f=>jan,l=>wieck'),('f=>tom,l=>lane');
INSERT 0 3
postgres=# select look_up_hstore(it,'f') from storit;look_up_hstore
----------------joshjantom
(3 rows)
I'll have to think of other ways to test it out, but right now it seems
to pass muster. I was a little unclear on what Python data structure an
ltree should produce.
Now if only we can work out the combinatorics issue ...
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Re: transforms
От:
Steve Singer <steve@ssinger.info>
Дата:
On 13-03-03 08:13 PM, Josh Berkus wrote: >> This (creating the extensions) works fine for me on a Ubuntu 10.x system >> > Now if only we can work out the combinatorics issue ... > The plpython2u extensions worked fine but I haven't been able to get this to work with plpython3u (python 3.1). create extension hstore_plpython3u; ERROR: could not load library "/usr/local/pgsql93git/lib/hstore_plpython3.so": /usr/local/pgsql93git/lib/hstore_plpython3.so: undefined symbol: _Py_NoneStruct Peter mentioned in the submission that the unit tests don't pass with python3, it doesn't work for meoutside the tests either. Steve
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> Peter mentioned in the submission that the unit tests don't pass with > python3, it doesn't work for meoutside the tests either. Also, note that the feature is the concept of Transforms, not the individual transforms which Peter provides. The idea is to enable a new kind of extension. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 3/3/13 6:14 PM, Josh Berkus wrote:
> Currently Peter is punting (as is proper in a new patch) by having a
> separate extension for each combination (hstore/plperl, hstore/plpython,
> ltree/plpython, etc.). This is obviously not a maintainable approach in
> the long run.
There is also a {Perl, Python, Ruby, etc.} binding for {libpq, libmysql,
libpng, libyaml, libssl, libgmp, etc.}, each as a separately
downloadable and buildable package. I don't think anyone has ever
seriously considered a system by which if, say, you have Python and
libyaml installed, pyyaml magically appears. Might be nice, but maybe
not. The solution, in practice, is that you download pyyaml, and it
pulls in any required dependencies. This would work the same way.
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter,
> There is also a {Perl, Python, Ruby, etc.} binding for {libpq, libmysql,
> libpng, libyaml, libssl, libgmp, etc.}, each as a separately
> downloadable and buildable package. I don't think anyone has ever
> seriously considered a system by which if, say, you have Python and
> libyaml installed, pyyaml magically appears. Might be nice, but maybe
> not. The solution, in practice, is that you download pyyaml, and it
> pulls in any required dependencies. This would work the same way.
That would be good too, although we don't currently have that
capability; if I try to install hstore_plpython without plpython
installed, it just errors out. Aside from that, what can we reasonably
do for 9.3 to get this feature in?
Maybe we add a transforms/ subdirectory of contrib, so that it can be as
crowded as we want? Or put the transforms on PGXN for now?
I want to see this feature go in so that the community starts writing
transforms this year instead of next year.
BTW, dependancies seem to be working OK for DROP:
postgres=# drop extension plpythonu;
ERROR: cannot drop extension plpythonu because other objects depend on it
DETAIL: extension hstore_plpythonu depends on extension plpythonufunction look_up_hstore(hstore,text) depends on transform for hstore
language plpythonuextension ltree_plpythonu depends on extension plpythonu
HINT: Use DROP ... CASCADE to drop the dependent objects too.
STATEMENT: drop extension plpythonu;
ERROR: cannot drop extension plpythonu because other objects depend on it
DETAIL: extension hstore_plpythonu depends on extension plpythonu
function look_up_hstore(hstore,text) depends on transform for hstore
language plpythonu
extension ltree_plpythonu depends on extension plpythonu
HINT: Use DROP ... CASCADE to drop the dependent objects too.
--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, I'm still getting intermittent linking failures: postgres=# drop extension plperl cascade; NOTICE: drop cascades to extension hstore_plperl DROP EXTENSION postgres=# create extension plperl; CREATE EXTENSION postgres=# create extension hstore_plperl; ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: hstoreUniquePairs STATEMENT: create extension hstore_plperl; ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: hstoreUniquePairs postgres=# There appears to be something wonky which breaks when I've been running 9.2, shut it down, and fire up 9.3. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> postgres=# create extension plperl; > CREATE EXTENSION > postgres=# create extension hstore_plperl; > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: > hstoreUniquePairs > STATEMENT: create extension hstore_plperl; > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: > hstoreUniquePairs > postgres=# > > There appears to be something wonky which breaks when I've been running > 9.2, shut it down, and fire up 9.3. More on this: the problem appears to be that the symbols for hstore are loaded only if I've just just created the extension in that database: postgres=# create database plperlh postgres-# ; CREATE DATABASE postgres=# \c plperlh; You are now connected to database "plperlh" as user "josh". plperlh=# create extension plperl; CREATE EXTENSION plperlh=# create extension hstore; CREATE EXTENSION plperlh=# create extension hstore_plperl; CREATE EXTENSION plperlh=# plperlh=# \c postgres You are now connected to database "postgres" as user "josh". postgres=# create extension hstore_plperl; ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: PL_thr_key STATEMENT: create extension hstore_plperl; ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: PL_thr_key -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
"Joshua D. Drake" <jd@commandprompt.com>
Дата:
On 03/05/2013 02:52 PM, Josh Berkus wrote: > plperlh=# \c postgres > You are now connected to database "postgres" as user "josh". > postgres=# create extension hstore_plperl; > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: > PL_thr_key > STATEMENT: create extension hstore_plperl; > ERROR: could not load library > "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: > PL_thr_key What happens if you set your LD_LIBRARY_PATH to reflect each installation before you start the database? JD > > > > > -- Command Prompt, Inc. - http://www.commandprompt.com/ PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, Postgres-XC @cmdpromptinc - 509-416-6579
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> What happens if you set your LD_LIBRARY_PATH to reflect each > installation before you start the database? No change, at least not setting it to $PGHOME/lib. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 3/5/13 5:52 PM, Josh Berkus wrote: > More on this: the problem appears to be that the symbols for hstore are > loaded only if I've just just created the extension in that database: I see. This is a problem with any kind of dynamically loadable module that uses another module. The other module is only loaded either at creation time or when a function from it is first used (or if a preload mechanism is used). At run time, this will sort itself out, because all the required modules will be loaded. The problem is when you create the "glue" extension and haven't invoked any code from any of the dependent extension in the session. Abstractly, the possible solutions are either not to check the functions when the extension is created (possibly settable by a flag) or to somehow force a load of all dependent extensions when the new extension is created. (I say extension here even though dynamically loadable modules are attached to functions, which makes this even more confusing.) In "normal" programming languages, this is normally addressed by placing explicit load/require/import statements before you do anything else. What we are doing here is more like an autoload functionality that some environments have. Those have the same problem.
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, > At run time, this will sort itself out, because all the required modules > will be loaded. The problem is when you create the "glue" extension and > haven't invoked any code from any of the dependent extension in the > session. Just invoking code doesn't seem to be enough. I tried just using the Hstore data type, and then loading hstore_plperl, but that still didn't work. It seems like only CREATE EXTENSION loads *all* the symbols. > Abstractly, the possible solutions are either not to check the > functions when the extension is created (possibly settable by a flag) or > to somehow force a load of all dependent extensions when the new > extension is created. The latter would be ideal, but I don't know that we currently have any mechanism for it. --Josh -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2013-03-06 09:53:29 -0800, Josh Berkus wrote: > Peter, > > > At run time, this will sort itself out, because all the required modules > > will be loaded. The problem is when you create the "glue" extension and > > haven't invoked any code from any of the dependent extension in the > > session. > > Just invoking code doesn't seem to be enough. I tried just using the > Hstore data type, and then loading hstore_plperl, but that still didn't > work. It seems like only CREATE EXTENSION loads *all* the symbols. Your error looks like youre erroring out in plperl not in hstore? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
> Your error looks like youre erroring out in plperl not in hstore? Look again. Peter, is there any way for you to tackle this issue? I have no idea how to fix it, myself ... -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2013-03-11 09:42:18 -0700, Josh Berkus wrote: > > > Your error looks like youre erroring out in plperl not in hstore? > > Look again. > ERROR: could not load library "/home/josh/pg93/lib/postgresql/hstore_plperl.so": > /home/josh/pg93/lib/postgresql/hstore_plperl.so: undefined symbol: PL_thr_key Thats a perl symbol. > Peter, is there any way for you to tackle this issue? I have no idea > how to fix it, myself ... If you add a: DO LANGUAGE plperlu $$$$; SELECT NULL::hstore; to the extension script, does it work? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
On 03/11/2013 09:50 AM, Andres Freund wrote: > DO LANGUAGE plperlu $$$$; > SELECT NULL::hstore; Aha, so that's what you meant! Yeah, it works if I reference both extensions before the CREATE EXTENSION. In that case, seems like that could be added to the extension SQL, no? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2013-03-11 09:55:34 -0700, Josh Berkus wrote: > On 03/11/2013 09:50 AM, Andres Freund wrote: > > DO LANGUAGE plperlu $$$$; > > SELECT NULL::hstore; > > Aha, so that's what you meant! > > Yeah, it works if I reference both extensions before the CREATE EXTENSION. > > In that case, seems like that could be added to the extension SQL, no? If we don't find a better solution, yes. Why don't we lookup type input/ouput function for parameters and return type during CREATE FUNCTION? That should solve the issue in a neater way? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: transforms
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Mon, 2013-03-11 at 18:11 +0100, Andres Freund wrote: > If we don't find a better solution, yes. Why don't we lookup type > input/ouput function for parameters and return type during CREATE > FUNCTION? That should solve the issue in a neater way? A function in general has no particular use for a type's input or output function. Also, a type's input/output functions are not necessarily in the same library as other things about that type that you might want or need.
Re: transforms
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2013-03-11 20:28:05 -0400, Peter Eisentraut wrote: > On Mon, 2013-03-11 at 18:11 +0100, Andres Freund wrote: > > If we don't find a better solution, yes. Why don't we lookup type > > input/ouput function for parameters and return type during CREATE > > FUNCTION? That should solve the issue in a neater way? > > A function in general has no particular use for a type's input or output > function. > Also, a type's input/output functions are not necessarily in the same > library as other things about that type that you might want or need. In theory they are not necessarily tied together, yes. In practice they nearly always are in the same shared object. The lookup for them afaics are the only reason why e.g. the plperl-hstore transform works outside of CREATE EXTENSION without explictly doing something ugly like: On 03/11/2013 09:50 AM, Andres Freund wrote: > DO LANGUAGE plperlu $$$$; > SELECT NULL::hstore; beforehand. If you have a better idea than putting the above into the extension script or making a function look up its parameters, I am all ears. It sure would only be a hack. Greetings, Andres Freund --Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: transforms
От:
Dimitri Fontaine <dimitri@2ndQuadrant.fr>
Дата:
Peter Eisentraut writes: > At run time, this will sort itself out, because all the required modules > will be loaded. The problem is when you create the "glue" extension and > haven't invoked any code from any of the dependent extension in the > session. Abstractly, the possible solutions are either not to check the > functions when the extension is created (possibly settable by a flag) or > to somehow force a load of all dependent extensions when the new > extension is created. (I say extension here even though dynamically > loadable modules are attached to functions, which makes this even more > confusing.) I think CREATE EXTENSION should be LOADing all distinct modules referenced from all required extensions functions. It does sound easy enough to code, and I can't imagine why we would want to not do that. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
On 03/13/2013 09:54 AM, Dimitri Fontaine wrote: > Peter Eisentraut writes: >> At run time, this will sort itself out, because all the required modules >> will be loaded. The problem is when you create the "glue" extension and >> haven't invoked any code from any of the dependent extension in the >> session. Abstractly, the possible solutions are either not to check the >> functions when the extension is created (possibly settable by a flag) or >> to somehow force a load of all dependent extensions when the new >> extension is created. (I say extension here even though dynamically >> loadable modules are attached to functions, which makes this even more >> confusing.) > > I think CREATE EXTENSION should be LOADing all distinct modules > referenced from all required extensions functions. It does sound easy > enough to code, and I can't imagine why we would want to not do that. Where are we with this? Will the loading issue be fixed, or should we bump this feature to 9.4? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, All: Does anyone feel like fixing the LOAD issue with transforms? I haven't seen any activity on the patch. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: transforms
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 6/12/13 1:20 AM, Josh Berkus wrote: > Peter, All: > > Does anyone feel like fixing the LOAD issue with transforms? I haven't > seen any activity on the patch. I plan to send in an updated patch.
[PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
A transform is an SQL object that supplies to functions for converting
between data types and procedural languages. For example, a transform
could arrange that hstore is converted to an appropriate hash or
dictionary object in PL/Perl or PL/Python.
Externally visible changes:
- new SQL commands CREATE TRANSFORM and DROP TRANSFORM
- system catalog pg_transform
- transform support in PL/Perl and PL/Python
- PL/Perl and PL/Python install their header files for use by external
types
- transforms contrib modules hstore_plperl, hstore_plpython,
ltree_plpython
- regression test mangling for Python 3 was moved to a separate
makefile, for use by extensions
The regression tests for general CREATE TRANSFORM functionality are
under the hstore_plperl module in order to be able to test it on a real
transform implementation, instead of having to create an entire fake
procedural language under src/test/regress/, say.
Dynamic module linking on OS X was changed to allow undefined symbols at
build time. This is necessary so that a transform module can use
symbols from the type and the language modules, if necessary. Other
platforms already behaved this way, but the default on OS X was
different.
---
Continued from 2013-01 commit fest. All known open issues have been fixed.
contrib/Makefile | 12 +
contrib/hstore_plperl/.gitignore | 4 +
contrib/hstore_plperl/Makefile | 23 ++
.../hstore_plperl/expected/create_transform.out | 70 +++++
contrib/hstore_plperl/expected/hstore_plperl.out | 170 ++++++++++++
contrib/hstore_plperl/hstore_plperl--1.0.sql | 17 ++
contrib/hstore_plperl/hstore_plperl.c | 90 ++++++
contrib/hstore_plperl/hstore_plperl.control | 6 +
contrib/hstore_plperl/hstore_plperlu--1.0.sql | 17 ++
contrib/hstore_plperl/hstore_plperlu.control | 6 +
contrib/hstore_plperl/sql/create_transform.sql | 45 +++
contrib/hstore_plperl/sql/hstore_plperl.sql | 120 ++++++++
contrib/hstore_plpython/.gitignore | 4 +
contrib/hstore_plpython/Makefile | 30 ++
.../hstore_plpython/expected/hstore_plpython.out | 137 +++++++++
contrib/hstore_plpython/hstore_plpython.c | 116 ++++++++
contrib/hstore_plpython/hstore_plpython2u--1.0.sql | 19 ++
contrib/hstore_plpython/hstore_plpython2u.control | 6 +
contrib/hstore_plpython/hstore_plpython3u--1.0.sql | 19 ++
contrib/hstore_plpython/hstore_plpython3u.control | 6 +
contrib/hstore_plpython/hstore_plpythonu--1.0.sql | 19 ++
contrib/hstore_plpython/hstore_plpythonu.control | 6 +
contrib/hstore_plpython/sql/hstore_plpython.sql | 106 +++++++
contrib/ltree_plpython/.gitignore | 4 +
contrib/ltree_plpython/Makefile | 30 ++
contrib/ltree_plpython/expected/ltree_plpython.out | 42 +++
contrib/ltree_plpython/ltree_plpython.c | 32 +++
contrib/ltree_plpython/ltree_plpython2u--1.0.sql | 12 +
contrib/ltree_plpython/ltree_plpython2u.control | 6 +
contrib/ltree_plpython/ltree_plpython3u--1.0.sql | 12 +
contrib/ltree_plpython/ltree_plpython3u.control | 6 +
contrib/ltree_plpython/ltree_plpythonu--1.0.sql | 12 +
contrib/ltree_plpython/ltree_plpythonu.control | 6 +
contrib/ltree_plpython/sql/ltree_plpython.sql | 34 +++
doc/src/sgml/catalogs.sgml | 73 +++++
doc/src/sgml/hstore.sgml | 18 ++
doc/src/sgml/ltree.sgml | 15 +
doc/src/sgml/ref/allfiles.sgml | 2 +
doc/src/sgml/ref/alter_extension.sgml | 21 ++
doc/src/sgml/ref/comment.sgml | 22 ++
doc/src/sgml/ref/create_transform.sgml | 187 +++++++++++++
doc/src/sgml/ref/drop_transform.sgml | 123 ++++++++
doc/src/sgml/reference.sgml | 2 +
src/Makefile.shlib | 2 +-
src/backend/catalog/Makefile | 1 +
src/backend/catalog/dependency.c | 8 +
src/backend/catalog/objectaddress.c | 76 ++++-
src/backend/catalog/pg_proc.c | 20 ++
src/backend/commands/dropcmds.c | 6 +
src/backend/commands/event_trigger.c | 3 +
src/backend/commands/functioncmds.c | 293 ++++++++++++++++++++
src/backend/nodes/copyfuncs.c | 17 ++
src/backend/nodes/equalfuncs.c | 15 +
src/backend/parser/gram.y | 82 +++++-
src/backend/parser/parse_func.c | 5 +
src/backend/tcop/utility.c | 16 ++
src/backend/utils/adt/ruleutils.c | 11 +-
src/backend/utils/cache/lsyscache.c | 83 ++++++
src/backend/utils/cache/syscache.c | 23 ++
src/bin/pg_dump/common.c | 5 +
src/bin/pg_dump/pg_dump.c | 230 +++++++++++++++
src/bin/pg_dump/pg_dump.h | 11 +
src/bin/pg_dump/pg_dump_sort.c | 13 +-
src/include/catalog/catversion.h | 2 +-
src/include/catalog/dependency.h | 1 +
src/include/catalog/indexing.h | 5 +
src/include/catalog/pg_transform.h | 47 ++++
src/include/commands/defrem.h | 3 +
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h | 15 +
src/include/parser/kwlist.h | 2 +
src/include/utils/lsyscache.h | 4 +
src/include/utils/syscache.h | 2 +
src/interfaces/ecpg/preproc/ecpg.tokens | 2 +-
src/interfaces/ecpg/preproc/ecpg.trailer | 5 +-
src/interfaces/ecpg/preproc/ecpg_keywords.c | 2 -
src/pl/plperl/GNUmakefile | 4 +-
src/pl/plperl/plperl.c | 32 ++-
src/pl/plperl/plperl_helpers.h | 2 +
src/pl/plpython/Makefile | 40 +--
src/pl/plpython/plpy_main.c | 1 +
src/pl/plpython/plpy_procedure.c | 6 +-
src/pl/plpython/plpy_procedure.h | 1 +
src/pl/plpython/plpy_spi.c | 3 +-
src/pl/plpython/plpy_typeio.c | 158 +++++++----
src/pl/plpython/plpy_typeio.h | 9 +-
src/pl/plpython/plpy_util.c | 21 +-
src/pl/plpython/plpy_util.h | 1 +
src/pl/plpython/plpython.h | 1 +
src/pl/plpython/regress-python3-mangle.mk | 35 +++
src/test/regress/expected/sanity_check.out | 3 +-
91 files changed, 2891 insertions(+), 144 deletions(-)
create mode 100644 contrib/hstore_plperl/.gitignore
create mode 100644 contrib/hstore_plperl/Makefile
create mode 100644 contrib/hstore_plperl/expected/create_transform.out
create mode 100644 contrib/hstore_plperl/expected/hstore_plperl.out
create mode 100644 contrib/hstore_plperl/hstore_plperl--1.0.sql
create mode 100644 contrib/hstore_plperl/hstore_plperl.c
create mode 100644 contrib/hstore_plperl/hstore_plperl.control
create mode 100644 contrib/hstore_plperl/hstore_plperlu--1.0.sql
create mode 100644 contrib/hstore_plperl/hstore_plperlu.control
create mode 100644 contrib/hstore_plperl/sql/create_transform.sql
create mode 100644 contrib/hstore_plperl/sql/hstore_plperl.sql
create mode 100644 contrib/hstore_plpython/.gitignore
create mode 100644 contrib/hstore_plpython/Makefile
create mode 100644 contrib/hstore_plpython/expected/hstore_plpython.out
create mode 100644 contrib/hstore_plpython/hstore_plpython.c
create mode 100644 contrib/hstore_plpython/hstore_plpython2u--1.0.sql
create mode 100644 contrib/hstore_plpython/hstore_plpython2u.control
create mode 100644 contrib/hstore_plpython/hstore_plpython3u--1.0.sql
create mode 100644 contrib/hstore_plpython/hstore_plpython3u.control
create mode 100644 contrib/hstore_plpython/hstore_plpythonu--1.0.sql
create mode 100644 contrib/hstore_plpython/hstore_plpythonu.control
create mode 100644 contrib/hstore_plpython/sql/hstore_plpython.sql
create mode 100644 contrib/ltree_plpython/.gitignore
create mode 100644 contrib/ltree_plpython/Makefile
create mode 100644 contrib/ltree_plpython/expected/ltree_plpython.out
create mode 100644 contrib/ltree_plpython/ltree_plpython.c
create mode 100644 contrib/ltree_plpython/ltree_plpython2u--1.0.sql
create mode 100644 contrib/ltree_plpython/ltree_plpython2u.control
create mode 100644 contrib/ltree_plpython/ltree_plpython3u--1.0.sql
create mode 100644 contrib/ltree_plpython/ltree_plpython3u.control
create mode 100644 contrib/ltree_plpython/ltree_plpythonu--1.0.sql
create mode 100644 contrib/ltree_plpython/ltree_plpythonu.control
create mode 100644 contrib/ltree_plpython/sql/ltree_plpython.sql
create mode 100644 doc/src/sgml/ref/create_transform.sgml
create mode 100644 doc/src/sgml/ref/drop_transform.sgml
create mode 100644 src/include/catalog/pg_transform.h
create mode 100644 src/pl/plpython/regress-python3-mangle.mk
diff --git a/contrib/Makefile b/contrib/Makefile
index 8a2a937..6495b13 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -79,6 +79,18 @@ else
ALWAYS_SUBDIRS += sepgsql
endif
+ifeq ($(with_perl),yes)
+SUBDIRS += hstore_plperl
+else
+ALWAYS_SUBDIRS += hstore_plperl
+endif
+
+ifeq ($(with_python),yes)
+SUBDIRS += hstore_plpython ltree_plpython
+else
+ALWAYS_SUBDIRS += hstore_plpython ltree_plpython
+endif
+
# Missing:
# start-scripts \ (does not have a makefile)
diff --git a/contrib/hstore_plperl/.gitignore b/contrib/hstore_plperl/.gitignore
new file mode 100644
index 0000000..5dcb3ff
--- /dev/null
+++ b/contrib/hstore_plperl/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/contrib/hstore_plperl/Makefile b/contrib/hstore_plperl/Makefile
new file mode 100644
index 0000000..81f037e
--- /dev/null
+++ b/contrib/hstore_plperl/Makefile
@@ -0,0 +1,23 @@
+# contrib/hstore_plperl/Makefile
+
+MODULE_big = hstore_plperl
+OBJS = hstore_plperl.o
+
+PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plperl -I$(perl_archlibexp)/CORE -I$(top_srcdir)/contrib/hstore
+
+EXTENSION = hstore_plperl hstore_plperlu
+DATA = hstore_plperl--1.0.sql hstore_plperlu--1.0.sql
+
+REGRESS = hstore_plperl create_transform
+REGRESS_OPTS = --extra-install=contrib/hstore --load-extension=hstore --load-extension=plperl --load-extension=plperlu
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/hstore_plperl
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/hstore_plperl/expected/create_transform.out b/contrib/hstore_plperl/expected/create_transform.out
new file mode 100644
index 0000000..8111d1b
--- /dev/null
+++ b/contrib/hstore_plperl/expected/create_transform.out
@@ -0,0 +1,70 @@
+-- general regression test for transforms
+DROP EXTENSION IF EXISTS hstore CASCADE;
+NOTICE: extension "hstore" does not exist, skipping
+DROP EXTENSION IF EXISTS plperl CASCADE;
+NOTICE: extension "plperl" does not exist, skipping
+DROP EXTENSION IF EXISTS hstore_plperl CASCADE;
+NOTICE: extension "hstore_plperl" does not exist, skipping
+CREATE EXTENSION hstore;
+CREATE EXTENSION plperl;
+CREATE FUNCTION hstore_to_plperl(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS '$libdir/hstore_plperl';
+CREATE FUNCTION plperl_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS '$libdir/hstore_plperl';
+CREATE TRANSFORM FOR foo LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+ERROR: type "foo" does not exist
+CREATE TRANSFORM FOR hstore LANGUAGE foo (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+ERROR: language "foo" does not exist
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_out(hstore), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+ERROR: return data type of FROM SQL function must be "internal"
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION internal_in(cstring), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+ERROR: first argument of transform function must be type "internal"
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+ERROR: transform for type hstore language plperl already exists
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal)); -- ok
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+DROP TRANSFORM FOR foo LANGUAGE plperl;
+ERROR: type "foo" does not exist
+DROP TRANSFORM FOR hstore LANGUAGE foo;
+ERROR: language "foo" does not exist
+DROP TRANSFORM FOR hstore LANGUAGE plperl;
+DROP TRANSFORM IF EXISTS FOR hstore LANGUAGE plperl;
+NOTICE: transform for type hstore language plperl does not exist, skipping
+DROP FUNCTION hstore_to_plperl(val internal);
+DROP FUNCTION plperl_to_hstore(val internal);
+CREATE EXTENSION hstore_plperl;
+\dx+ hstore_plperl
+ Objects in extension "hstore_plperl"
+ Object Description
+--------------------------------------
+ function hstore_to_plperl(internal)
+ function plperl_to_hstore(internal)
+ transform for hstore language plperl
+(3 rows)
+
+ALTER EXTENSION hstore_plperl DROP TRANSFORM FOR hstore LANGUAGE plperl;
+\dx+ hstore_plperl
+Objects in extension "hstore_plperl"
+ Object Description
+-------------------------------------
+ function hstore_to_plperl(internal)
+ function plperl_to_hstore(internal)
+(2 rows)
+
+ALTER EXTENSION hstore_plperl ADD TRANSFORM FOR hstore LANGUAGE plperl;
+\dx+ hstore_plperl
+ Objects in extension "hstore_plperl"
+ Object Description
+--------------------------------------
+ function hstore_to_plperl(internal)
+ function plperl_to_hstore(internal)
+ transform for hstore language plperl
+(3 rows)
+
+DROP EXTENSION hstore CASCADE;
+NOTICE: drop cascades to extension hstore_plperl
+DROP EXTENSION plperl CASCADE;
diff --git a/contrib/hstore_plperl/expected/hstore_plperl.out b/contrib/hstore_plperl/expected/hstore_plperl.out
new file mode 100644
index 0000000..ce31ab8
--- /dev/null
+++ b/contrib/hstore_plperl/expected/hstore_plperl.out
@@ -0,0 +1,170 @@
+CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperlu;
+-- test hstore -> perl
+CREATE FUNCTION test1(val hstore) RETURNS int
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_[0]));
+return scalar(keys %{$_[0]});
+$$;
+SELECT test1('aa=>bb, cc=>NULL'::hstore);
+INFO: $VAR1 = {
+ 'cc' => undef,
+ 'aa' => 'bb'
+ };
+
+CONTEXT: PL/Perl function "test1"
+ test1
+-------
+ 2
+(1 row)
+
+-- test hstore[] -> perl
+CREATE FUNCTION test1arr(val hstore[]) RETURNS int
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_[0]->[0], $_[0]->[1]));
+return scalar(keys %{$_[0]});
+$$;
+SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
+INFO: $VAR1 = {
+ 'cc' => undef,
+ 'aa' => 'bb'
+ };
+$VAR2 = {
+ 'dd' => 'ee'
+ };
+
+CONTEXT: PL/Perl function "test1arr"
+ test1arr
+----------
+ 2
+(1 row)
+
+-- test perl -> hstore
+CREATE FUNCTION test2() RETURNS hstore
+LANGUAGE plperl
+AS $$
+$val = {a => 1, b => 'boo', c => undef};
+return $val;
+$$;
+SELECT test2();
+ test2
+---------------------------------
+ "a"=>"1", "b"=>"boo", "c"=>NULL
+(1 row)
+
+-- test perl -> hstore[]
+CREATE FUNCTION test2arr() RETURNS hstore[]
+LANGUAGE plperl
+AS $$
+$val = [{a => 1, b => 'boo', c => undef}, {d => 2}];
+return $val;
+$$;
+SELECT test2arr();
+ test2arr
+--------------------------------------------------------------
+ {"\"a\"=>\"1\", \"b\"=>\"boo\", \"c\"=>NULL","\"d\"=>\"2\""}
+(1 row)
+
+-- test as part of prepare/execute
+CREATE FUNCTION test3() RETURNS void
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+
+$rv = spi_exec_query(q{SELECT 'aa=>bb, cc=>NULL'::hstore AS col1});
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+
+$val = {a => 1, b => 'boo', c => undef};
+$plan = spi_prepare(q{SELECT $1::text AS col1}, "hstore");
+$rv = spi_exec_prepared($plan, {}, $val);
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+$$;
+SELECT test3();
+INFO: $VAR1 = {
+ 'cc' => undef,
+ 'aa' => 'bb'
+ };
+
+CONTEXT: PL/Perl function "test3"
+INFO: $VAR1 = '"a"=>"1", "b"=>"boo", "c"=>NULL';
+
+CONTEXT: PL/Perl function "test3"
+ test3
+-------
+
+(1 row)
+
+-- test inline
+DO LANGUAGE plperlu $$
+use Data::Dumper;
+
+$rv = spi_exec_query(q{SELECT 'aa=>bb, cc=>NULL'::hstore AS col1});
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+
+$val = {a => 1, b => 'boo', c => undef};
+$plan = spi_prepare(q{SELECT $1::text AS col1}, "hstore");
+$rv = spi_exec_prepared($plan, {}, $val);
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+$$;
+INFO: $VAR1 = {
+ 'cc' => undef,
+ 'aa' => 'bb'
+ };
+
+CONTEXT: PL/Perl anonymous code block
+INFO: $VAR1 = '"a"=>"1", "b"=>"boo", "c"=>NULL';
+
+CONTEXT: PL/Perl anonymous code block
+-- test trigger
+CREATE TABLE test1 (a int, b hstore);
+INSERT INTO test1 VALUES (1, 'aa=>bb, cc=>NULL');
+SELECT * FROM test1;
+ a | b
+---+------------------------
+ 1 | "aa"=>"bb", "cc"=>NULL
+(1 row)
+
+CREATE FUNCTION test4() RETURNS trigger
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_TD->{new}));
+if ($_TD->{new}{a} == 1) {
+ $_TD->{new}{b} = {a => 1, b => 'boo', c => undef};
+}
+
+return "MODIFY";
+$$;
+CREATE TRIGGER test4 BEFORE UPDATE ON test1 FOR EACH ROW EXECUTE PROCEDURE test4();
+UPDATE test1 SET a = a;
+INFO: $VAR1 = {
+ 'a' => '1',
+ 'b' => {
+ 'cc' => undef,
+ 'aa' => 'bb'
+ }
+ };
+
+CONTEXT: PL/Perl function "test4"
+SELECT * FROM test1;
+ a | b
+---+---------------------------------
+ 1 | "a"=>"1", "b"=>"boo", "c"=>NULL
+(1 row)
+
+DROP TABLE test1;
+DROP FUNCTION test1(hstore);
+DROP FUNCTION test1arr(hstore[]);
+DROP FUNCTION test2();
+DROP FUNCTION test2arr();
+DROP FUNCTION test3();
+DROP FUNCTION test4();
+DROP EXTENSION hstore_plperl;
+DROP EXTENSION hstore_plperlu;
+DROP EXTENSION hstore;
+DROP EXTENSION plperl;
+DROP EXTENSION plperlu;
diff --git a/contrib/hstore_plperl/hstore_plperl--1.0.sql b/contrib/hstore_plperl/hstore_plperl--1.0.sql
new file mode 100644
index 0000000..ea0ad76
--- /dev/null
+++ b/contrib/hstore_plperl/hstore_plperl--1.0.sql
@@ -0,0 +1,17 @@
+-- make sure the prerequisite libraries are loaded
+DO '' LANGUAGE plperl;
+SELECT NULL::hstore;
+
+
+CREATE FUNCTION hstore_to_plperl(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME';
+
+CREATE FUNCTION plperl_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME';
+
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (
+ FROM SQL WITH FUNCTION hstore_to_plperl(internal),
+ TO SQL WITH FUNCTION plperl_to_hstore(internal)
+);
diff --git a/contrib/hstore_plperl/hstore_plperl.c b/contrib/hstore_plperl/hstore_plperl.c
new file mode 100644
index 0000000..cdc224c
--- /dev/null
+++ b/contrib/hstore_plperl/hstore_plperl.c
@@ -0,0 +1,90 @@
+#include "postgres.h"
+#undef _
+#include "fmgr.h"
+#include "plperl.h"
+#include "plperl_helpers.h"
+#include "hstore.h"
+
+PG_MODULE_MAGIC;
+
+
+PG_FUNCTION_INFO_V1(hstore_to_plperl);
+Datum hstore_to_plperl(PG_FUNCTION_ARGS);
+
+Datum
+hstore_to_plperl(PG_FUNCTION_ARGS)
+{
+ HStore *in = PG_GETARG_HS(0);
+ int i;
+ int count = HS_COUNT(in);
+ char *base = STRPTR(in);
+ HEntry *entries = ARRPTR(in);
+ HV *hv;
+
+ hv = newHV();
+
+ for (i = 0; i < count; i++)
+ {
+ const char *key;
+ SV *value;
+
+ key = pnstrdup(HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
+ value = HS_VALISNULL(entries, i) ? newSV(0) : cstr2sv(pnstrdup(HS_VAL(entries, base,i), HS_VALLEN(entries, i)));
+
+ (void) hv_store(hv, key, strlen(key), value, 0);
+ }
+
+ return PointerGetDatum(newRV((SV *) hv));
+}
+
+
+PG_FUNCTION_INFO_V1(plperl_to_hstore);
+Datum plperl_to_hstore(PG_FUNCTION_ARGS);
+
+Datum
+plperl_to_hstore(PG_FUNCTION_ARGS)
+{
+ HV *hv;
+ HE *he;
+ int32 buflen;
+ int32 i;
+ int32 pcount;
+ HStore *out;
+ Pairs *pairs;
+
+ hv = (HV *) SvRV((SV *) PG_GETARG_POINTER(0));
+
+ pcount = hv_iterinit(hv);
+
+ pairs = palloc(pcount * sizeof(Pairs));
+
+ i = 0;
+ while ((he = hv_iternext(hv)))
+ {
+ char *key = sv2cstr(HeSVKEY_force(he));
+ SV *value = HeVAL(he);
+
+ pairs[i].key = pstrdup(key);
+ pairs[i].keylen = hstoreCheckKeyLen(strlen(pairs[i].key));
+ pairs[i].needfree = true;
+
+ if (!SvOK(value))
+ {
+ pairs[i].val = NULL;
+ pairs[i].vallen = 0;
+ pairs[i].isnull = true;
+ }
+ else
+ {
+ pairs[i].val = pstrdup(sv2cstr(value));
+ pairs[i].vallen = hstoreCheckValLen(strlen(pairs[i].val));
+ pairs[i].isnull = false;
+ }
+
+ i++;
+ }
+
+ pcount = hstoreUniquePairs(pairs, pcount, &buflen);
+ out = hstorePairs(pairs, pcount, buflen);
+ PG_RETURN_POINTER(out);
+}
diff --git a/contrib/hstore_plperl/hstore_plperl.control b/contrib/hstore_plperl/hstore_plperl.control
new file mode 100644
index 0000000..16277f6
--- /dev/null
+++ b/contrib/hstore_plperl/hstore_plperl.control
@@ -0,0 +1,6 @@
+# hstore_plperl extension
+comment = 'transform between hstore and plperl'
+default_version = '1.0'
+module_pathname = '$libdir/hstore_plperl'
+relocatable = true
+requires = 'hstore,plperl'
diff --git a/contrib/hstore_plperl/hstore_plperlu--1.0.sql b/contrib/hstore_plperl/hstore_plperlu--1.0.sql
new file mode 100644
index 0000000..46ad35c
--- /dev/null
+++ b/contrib/hstore_plperl/hstore_plperlu--1.0.sql
@@ -0,0 +1,17 @@
+-- make sure the prerequisite libraries are loaded
+DO '' LANGUAGE plperlu;
+SELECT NULL::hstore;
+
+
+CREATE FUNCTION hstore_to_plperlu(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'hstore_to_plperl';
+
+CREATE FUNCTION plperlu_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'plperl_to_hstore';
+
+CREATE TRANSFORM FOR hstore LANGUAGE plperlu (
+ FROM SQL WITH FUNCTION hstore_to_plperlu(internal),
+ TO SQL WITH FUNCTION plperlu_to_hstore(internal)
+);
diff --git a/contrib/hstore_plperl/hstore_plperlu.control b/contrib/hstore_plperl/hstore_plperlu.control
new file mode 100644
index 0000000..c8d43b4
--- /dev/null
+++ b/contrib/hstore_plperl/hstore_plperlu.control
@@ -0,0 +1,6 @@
+# hstore_plperlu extension
+comment = 'transform between hstore and plperlu'
+default_version = '1.0'
+module_pathname = '$libdir/hstore_plperl'
+relocatable = true
+requires = 'hstore,plperlu'
diff --git a/contrib/hstore_plperl/sql/create_transform.sql b/contrib/hstore_plperl/sql/create_transform.sql
new file mode 100644
index 0000000..4f615e7
--- /dev/null
+++ b/contrib/hstore_plperl/sql/create_transform.sql
@@ -0,0 +1,45 @@
+-- general regression test for transforms
+
+DROP EXTENSION IF EXISTS hstore CASCADE;
+DROP EXTENSION IF EXISTS plperl CASCADE;
+DROP EXTENSION IF EXISTS hstore_plperl CASCADE;
+
+CREATE EXTENSION hstore;
+CREATE EXTENSION plperl;
+
+CREATE FUNCTION hstore_to_plperl(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS '$libdir/hstore_plperl';
+
+CREATE FUNCTION plperl_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS '$libdir/hstore_plperl';
+
+CREATE TRANSFORM FOR foo LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+CREATE TRANSFORM FOR hstore LANGUAGE foo (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_out(hstore), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION internal_in(cstring), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+CREATE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- fail
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal), TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (FROM SQL WITH FUNCTION hstore_to_plperl(internal)); -- ok
+CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plperl (TO SQL WITH FUNCTION plperl_to_hstore(internal)); -- ok
+
+DROP TRANSFORM FOR foo LANGUAGE plperl;
+DROP TRANSFORM FOR hstore LANGUAGE foo;
+DROP TRANSFORM FOR hstore LANGUAGE plperl;
+DROP TRANSFORM IF EXISTS FOR hstore LANGUAGE plperl;
+
+DROP FUNCTION hstore_to_plperl(val internal);
+DROP FUNCTION plperl_to_hstore(val internal);
+
+CREATE EXTENSION hstore_plperl;
+\dx+ hstore_plperl
+ALTER EXTENSION hstore_plperl DROP TRANSFORM FOR hstore LANGUAGE plperl;
+\dx+ hstore_plperl
+ALTER EXTENSION hstore_plperl ADD TRANSFORM FOR hstore LANGUAGE plperl;
+\dx+ hstore_plperl
+
+DROP EXTENSION hstore CASCADE;
+DROP EXTENSION plperl CASCADE;
diff --git a/contrib/hstore_plperl/sql/hstore_plperl.sql b/contrib/hstore_plperl/sql/hstore_plperl.sql
new file mode 100644
index 0000000..4796050
--- /dev/null
+++ b/contrib/hstore_plperl/sql/hstore_plperl.sql
@@ -0,0 +1,120 @@
+CREATE EXTENSION hstore_plperl;
+CREATE EXTENSION hstore_plperlu;
+
+
+-- test hstore -> perl
+CREATE FUNCTION test1(val hstore) RETURNS int
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_[0]));
+return scalar(keys %{$_[0]});
+$$;
+
+SELECT test1('aa=>bb, cc=>NULL'::hstore);
+
+
+-- test hstore[] -> perl
+CREATE FUNCTION test1arr(val hstore[]) RETURNS int
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_[0]->[0], $_[0]->[1]));
+return scalar(keys %{$_[0]});
+$$;
+
+SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
+
+
+-- test perl -> hstore
+CREATE FUNCTION test2() RETURNS hstore
+LANGUAGE plperl
+AS $$
+$val = {a => 1, b => 'boo', c => undef};
+return $val;
+$$;
+
+SELECT test2();
+
+
+-- test perl -> hstore[]
+CREATE FUNCTION test2arr() RETURNS hstore[]
+LANGUAGE plperl
+AS $$
+$val = [{a => 1, b => 'boo', c => undef}, {d => 2}];
+return $val;
+$$;
+
+SELECT test2arr();
+
+
+-- test as part of prepare/execute
+CREATE FUNCTION test3() RETURNS void
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+
+$rv = spi_exec_query(q{SELECT 'aa=>bb, cc=>NULL'::hstore AS col1});
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+
+$val = {a => 1, b => 'boo', c => undef};
+$plan = spi_prepare(q{SELECT $1::text AS col1}, "hstore");
+$rv = spi_exec_prepared($plan, {}, $val);
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+$$;
+
+SELECT test3();
+
+
+-- test inline
+DO LANGUAGE plperlu $$
+use Data::Dumper;
+
+$rv = spi_exec_query(q{SELECT 'aa=>bb, cc=>NULL'::hstore AS col1});
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+
+$val = {a => 1, b => 'boo', c => undef};
+$plan = spi_prepare(q{SELECT $1::text AS col1}, "hstore");
+$rv = spi_exec_prepared($plan, {}, $val);
+elog(INFO, Dumper($rv->{rows}[0]->{col1}));
+$$;
+
+
+-- test trigger
+CREATE TABLE test1 (a int, b hstore);
+INSERT INTO test1 VALUES (1, 'aa=>bb, cc=>NULL');
+SELECT * FROM test1;
+
+CREATE FUNCTION test4() RETURNS trigger
+LANGUAGE plperlu
+AS $$
+use Data::Dumper;
+elog(INFO, Dumper($_TD->{new}));
+if ($_TD->{new}{a} == 1) {
+ $_TD->{new}{b} = {a => 1, b => 'boo', c => undef};
+}
+
+return "MODIFY";
+$$;
+
+CREATE TRIGGER test4 BEFORE UPDATE ON test1 FOR EACH ROW EXECUTE PROCEDURE test4();
+
+UPDATE test1 SET a = a;
+SELECT * FROM test1;
+
+
+DROP TABLE test1;
+
+DROP FUNCTION test1(hstore);
+DROP FUNCTION test1arr(hstore[]);
+DROP FUNCTION test2();
+DROP FUNCTION test2arr();
+DROP FUNCTION test3();
+DROP FUNCTION test4();
+
+
+DROP EXTENSION hstore_plperl;
+DROP EXTENSION hstore_plperlu;
+DROP EXTENSION hstore;
+DROP EXTENSION plperl;
+DROP EXTENSION plperlu;
diff --git a/contrib/hstore_plpython/.gitignore b/contrib/hstore_plpython/.gitignore
new file mode 100644
index 0000000..5dcb3ff
--- /dev/null
+++ b/contrib/hstore_plpython/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/contrib/hstore_plpython/Makefile b/contrib/hstore_plpython/Makefile
new file mode 100644
index 0000000..65ddcf7
--- /dev/null
+++ b/contrib/hstore_plpython/Makefile
@@ -0,0 +1,30 @@
+# contrib/hstore_plpython/Makefile
+
+MODULE_big = hstore_plpython$(python_majorversion)
+OBJS = hstore_plpython.o
+
+PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/hstore
+
+EXTENSION = hstore_plpythonu hstore_plpython2u hstore_plpython3u
+DATA = hstore_plpythonu--1.0.sql hstore_plpython2u--1.0.sql hstore_plpython3u--1.0.sql
+
+REGRESS = hstore_plpython
+REGRESS_PLPYTHON3_MANGLE := $(REGRESS)
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/hstore_plpython
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+REGRESS_OPTS = --extra-install=contrib/hstore --load-extension=hstore
+ifeq ($(python_majorversion),2)
+REGRESS_OPTS += --load-extension=plpythonu --load-extension=hstore_plpythonu
+endif
+
+include $(top_srcdir)/src/pl/plpython/regress-python3-mangle.mk
diff --git a/contrib/hstore_plpython/expected/hstore_plpython.out b/contrib/hstore_plpython/expected/hstore_plpython.out
new file mode 100644
index 0000000..1bc59dd
--- /dev/null
+++ b/contrib/hstore_plpython/expected/hstore_plpython.out
@@ -0,0 +1,137 @@
+CREATE EXTENSION plpython2u;
+CREATE EXTENSION hstore_plpython2u;
+-- test hstore -> python
+CREATE FUNCTION test1(val hstore) RETURNS int
+LANGUAGE plpythonu
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+SELECT test1('aa=>bb, cc=>NULL'::hstore);
+INFO: {'aa': 'bb', 'cc': None}
+CONTEXT: PL/Python function "test1"
+ test1
+-------
+ 2
+(1 row)
+
+-- the same with the versioned language name
+CREATE FUNCTION test1n(val hstore) RETURNS int
+LANGUAGE plpython2u
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+SELECT test1n('aa=>bb, cc=>NULL'::hstore);
+INFO: {'aa': 'bb', 'cc': None}
+CONTEXT: PL/Python function "test1n"
+ test1n
+--------
+ 2
+(1 row)
+
+-- test hstore[] -> python
+ CREATE FUNCTION test1arr(val hstore[]) RETURNS int
+ LANGUAGE plpythonu
+ AS $$
+ plpy.info(repr(val))
+ return len(val)
+ $$;
+ SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
+INFO: [{'aa': 'bb', 'cc': None}, {'dd': 'ee'}]
+CONTEXT: PL/Python function "test1arr"
+ test1arr
+----------
+ 2
+(1 row)
+
+-- test python -> hstore
+CREATE FUNCTION test2() RETURNS hstore
+LANGUAGE plpythonu
+AS $$
+val = {'a': 1, 'b': 'boo', 'c': None}
+return val
+$$;
+SELECT test2();
+ test2
+---------------------------------
+ "a"=>"1", "b"=>"boo", "c"=>NULL
+(1 row)
+
+-- test python -> hstore[]
+ CREATE FUNCTION test2arr() RETURNS hstore[]
+ LANGUAGE plpythonu
+ AS $$
+ val = [{'a': 1, 'b': 'boo', 'c': None}, {'d': 2}]
+ return val
+ $$;
+ SELECT test2arr();
+ test2arr
+--------------------------------------------------------------
+ {"\"a\"=>\"1\", \"b\"=>\"boo\", \"c\"=>NULL","\"d\"=>\"2\""}
+(1 row)
+
+-- test as part of prepare/execute
+CREATE FUNCTION test3() RETURNS void
+LANGUAGE plpythonu
+AS $$
+rv = plpy.execute("SELECT 'aa=>bb, cc=>NULL'::hstore AS col1")
+plpy.info(repr(rv[0]["col1"]))
+
+val = {'a': 1, 'b': 'boo', 'c': None}
+plan = plpy.prepare("SELECT $1::text AS col1", ["hstore"])
+rv = plpy.execute(plan, [val])
+plpy.info(repr(rv[0]["col1"]))
+$$;
+SELECT test3();
+INFO: {'aa': 'bb', 'cc': None}
+CONTEXT: PL/Python function "test3"
+INFO: '"a"=>"1", "b"=>"boo", "c"=>NULL'
+CONTEXT: PL/Python function "test3"
+ test3
+-------
+
+(1 row)
+
+-- test inline
+DO LANGUAGE plpythonu $$
+rv = plpy.execute("SELECT 'aa=>bb, cc=>NULL'::hstore AS col1")
+plpy.info(repr(rv[0]["col1"]))
+
+val = {'a': 1, 'b': 'boo', 'c': None}
+plan = plpy.prepare("SELECT $1::text AS col1", ["hstore"])
+rv = plpy.execute(plan, [val])
+plpy.info(repr(rv[0]["col1"]))
+$$;
+INFO: {'aa': 'bb', 'cc': None}
+CONTEXT: PL/Python anonymous code block
+INFO: '"a"=>"1", "b"=>"boo", "c"=>NULL'
+CONTEXT: PL/Python anonymous code block
+-- test trigger
+CREATE TABLE test1 (a int, b hstore);
+INSERT INTO test1 VALUES (1, 'aa=>bb, cc=>NULL');
+SELECT * FROM test1;
+ a | b
+---+------------------------
+ 1 | "aa"=>"bb", "cc"=>NULL
+(1 row)
+
+CREATE FUNCTION test4() RETURNS trigger
+LANGUAGE plpythonu
+AS $$
+plpy.info("Trigger row: %r" % TD["new"])
+if TD["new"]["a"] == 1:
+ TD["new"]["b"] = {'a': 1, 'b': 'boo', 'c': None}
+
+return "MODIFY"
+$$;
+CREATE TRIGGER test4 BEFORE UPDATE ON test1 FOR EACH ROW EXECUTE PROCEDURE test4();
+UPDATE test1 SET a = a;
+INFO: Trigger row: {'a': 1, 'b': {'aa': 'bb', 'cc': None}}
+CONTEXT: PL/Python function "test4"
+SELECT * FROM test1;
+ a | b
+---+---------------------------------
+ 1 | "a"=>"1", "b"=>"boo", "c"=>NULL
+(1 row)
+
diff --git a/contrib/hstore_plpython/hstore_plpython.c b/contrib/hstore_plpython/hstore_plpython.c
new file mode 100644
index 0000000..92cd4f8
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpython.c
@@ -0,0 +1,116 @@
+#include "postgres.h"
+#include "fmgr.h"
+#include "plpython.h"
+#include "plpy_typeio.h"
+#include "hstore.h"
+
+PG_MODULE_MAGIC;
+
+
+PG_FUNCTION_INFO_V1(hstore_to_plpython);
+Datum hstore_to_plpython(PG_FUNCTION_ARGS);
+
+Datum
+hstore_to_plpython(PG_FUNCTION_ARGS)
+{
+ HStore *in = PG_GETARG_HS(0);
+ int i;
+ int count = HS_COUNT(in);
+ char *base = STRPTR(in);
+ HEntry *entries = ARRPTR(in);
+ PyObject *dict;
+
+ dict = PyDict_New();
+
+ for (i = 0; i < count; i++)
+ {
+ PyObject *key;
+
+ key = PyString_FromStringAndSize(HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
+ if (HS_VALISNULL(entries, i))
+ PyDict_SetItem(dict, key, Py_None);
+ else
+ {
+ PyObject *value;
+
+ value = PyString_FromStringAndSize(HS_VAL(entries, base,i), HS_VALLEN(entries, i));
+ PyDict_SetItem(dict, key, value);
+ Py_XDECREF(value);
+ }
+ Py_XDECREF(key);
+ }
+
+ return PointerGetDatum(dict);
+}
+
+
+PG_FUNCTION_INFO_V1(plpython_to_hstore);
+Datum plpython_to_hstore(PG_FUNCTION_ARGS);
+
+Datum
+plpython_to_hstore(PG_FUNCTION_ARGS)
+{
+ PyObject *dict;
+ volatile PyObject *items_v = NULL;
+ int32 pcount;
+ HStore *out;
+
+ dict = (PyObject *) PG_GETARG_POINTER(0);
+ if (!PyMapping_Check(dict))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("not a Python mapping")));
+
+ pcount = PyMapping_Size(dict);
+ items_v = PyMapping_Items(dict);
+
+ PG_TRY();
+ {
+ int32 buflen;
+ int32 i;
+ Pairs *pairs;
+ PyObject *items = (PyObject *) items_v;
+
+ pairs = palloc(pcount * sizeof(*pairs));
+
+ for (i = 0; i < pcount; i++)
+ {
+ PyObject *tuple;
+ PyObject *key;
+ PyObject *value;
+
+ tuple = PyList_GetItem(items, i);
+ key = PyTuple_GetItem(tuple, 0);
+ value = PyTuple_GetItem(tuple, 1);
+
+ pairs[i].key = PLyObject_AsString(key);
+ pairs[i].keylen = hstoreCheckKeyLen(strlen(pairs[i].key));
+ pairs[i].needfree = true;
+
+ if (value == Py_None)
+ {
+ pairs[i].val = NULL;
+ pairs[i].vallen = 0;
+ pairs[i].isnull = true;
+ }
+ else
+ {
+ pairs[i].val = PLyObject_AsString(value);
+ pairs[i].vallen = hstoreCheckValLen(strlen(pairs[i].val));
+ pairs[i].isnull = false;
+ }
+ }
+ Py_DECREF(items_v);
+
+ pcount = hstoreUniquePairs(pairs, pcount, &buflen);
+ out = hstorePairs(pairs, pcount, buflen);
+ }
+ PG_CATCH();
+ {
+ Py_DECREF(items_v);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
+
+ PG_RETURN_POINTER(out);
+}
diff --git a/contrib/hstore_plpython/hstore_plpython2u--1.0.sql b/contrib/hstore_plpython/hstore_plpython2u--1.0.sql
new file mode 100644
index 0000000..c998de5
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpython2u--1.0.sql
@@ -0,0 +1,19 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpython2u;
+SELECT NULL::hstore;
+
+
+CREATE FUNCTION hstore_to_plpython2(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'hstore_to_plpython';
+
+CREATE FUNCTION plpython2_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'plpython_to_hstore';
+
+CREATE TRANSFORM FOR hstore LANGUAGE plpython2u (
+ FROM SQL WITH FUNCTION hstore_to_plpython2(internal),
+ TO SQL WITH FUNCTION plpython2_to_hstore(internal)
+);
+
+COMMENT ON TRANSFORM FOR hstore LANGUAGE plpython2u IS 'transform between hstore and Python dict';
diff --git a/contrib/hstore_plpython/hstore_plpython2u.control b/contrib/hstore_plpython/hstore_plpython2u.control
new file mode 100644
index 0000000..ed90567
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpython2u.control
@@ -0,0 +1,6 @@
+# hstore_plpython2u extension
+comment = 'transform between hstore and plpython2u'
+default_version = '1.0'
+module_pathname = '$libdir/hstore_plpython2'
+relocatable = true
+requires = 'hstore,plpython2u'
diff --git a/contrib/hstore_plpython/hstore_plpython3u--1.0.sql b/contrib/hstore_plpython/hstore_plpython3u--1.0.sql
new file mode 100644
index 0000000..61d0e47
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpython3u--1.0.sql
@@ -0,0 +1,19 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpython3u;
+SELECT NULL::hstore;
+
+
+CREATE FUNCTION hstore_to_plpython3(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'hstore_to_plpython';
+
+CREATE FUNCTION plpython3_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'plpython_to_hstore';
+
+CREATE TRANSFORM FOR hstore LANGUAGE plpython3u (
+ FROM SQL WITH FUNCTION hstore_to_plpython3(internal),
+ TO SQL WITH FUNCTION plpython3_to_hstore(internal)
+);
+
+COMMENT ON TRANSFORM FOR hstore LANGUAGE plpython3u IS 'transform between hstore and Python dict';
diff --git a/contrib/hstore_plpython/hstore_plpython3u.control b/contrib/hstore_plpython/hstore_plpython3u.control
new file mode 100644
index 0000000..d86f38e
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpython3u.control
@@ -0,0 +1,6 @@
+# hstore_plpython3u extension
+comment = 'transform between hstore and plpython3u'
+default_version = '1.0'
+module_pathname = '$libdir/hstore_plpython3'
+relocatable = true
+requires = 'hstore,plpython3u'
diff --git a/contrib/hstore_plpython/hstore_plpythonu--1.0.sql b/contrib/hstore_plpython/hstore_plpythonu--1.0.sql
new file mode 100644
index 0000000..6acb97a
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpythonu--1.0.sql
@@ -0,0 +1,19 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpythonu;
+SELECT NULL::hstore;
+
+
+CREATE FUNCTION hstore_to_plpython(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME';
+
+CREATE FUNCTION plpython_to_hstore(val internal) RETURNS hstore
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME';
+
+CREATE TRANSFORM FOR hstore LANGUAGE plpythonu (
+ FROM SQL WITH FUNCTION hstore_to_plpython(internal),
+ TO SQL WITH FUNCTION plpython_to_hstore(internal)
+);
+
+COMMENT ON TRANSFORM FOR hstore LANGUAGE plpythonu IS 'transform between hstore and Python dict';
diff --git a/contrib/hstore_plpython/hstore_plpythonu.control b/contrib/hstore_plpython/hstore_plpythonu.control
new file mode 100644
index 0000000..8e9b35e
--- /dev/null
+++ b/contrib/hstore_plpython/hstore_plpythonu.control
@@ -0,0 +1,6 @@
+# hstore_plpythonu extension
+comment = 'transform between hstore and plpythonu'
+default_version = '1.0'
+module_pathname = '$libdir/hstore_plpython2'
+relocatable = true
+requires = 'hstore,plpythonu'
diff --git a/contrib/hstore_plpython/sql/hstore_plpython.sql b/contrib/hstore_plpython/sql/hstore_plpython.sql
new file mode 100644
index 0000000..566cdc5
--- /dev/null
+++ b/contrib/hstore_plpython/sql/hstore_plpython.sql
@@ -0,0 +1,106 @@
+CREATE EXTENSION plpython2u;
+CREATE EXTENSION hstore_plpython2u;
+
+
+-- test hstore -> python
+CREATE FUNCTION test1(val hstore) RETURNS int
+LANGUAGE plpythonu
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+
+SELECT test1('aa=>bb, cc=>NULL'::hstore);
+
+
+-- the same with the versioned language name
+CREATE FUNCTION test1n(val hstore) RETURNS int
+LANGUAGE plpython2u
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+
+SELECT test1n('aa=>bb, cc=>NULL'::hstore);
+
+
+-- test hstore[] -> python
+ CREATE FUNCTION test1arr(val hstore[]) RETURNS int
+ LANGUAGE plpythonu
+ AS $$
+ plpy.info(repr(val))
+ return len(val)
+ $$;
+
+ SELECT test1arr(array['aa=>bb, cc=>NULL'::hstore, 'dd=>ee']);
+
+
+-- test python -> hstore
+CREATE FUNCTION test2() RETURNS hstore
+LANGUAGE plpythonu
+AS $$
+val = {'a': 1, 'b': 'boo', 'c': None}
+return val
+$$;
+
+SELECT test2();
+
+
+-- test python -> hstore[]
+ CREATE FUNCTION test2arr() RETURNS hstore[]
+ LANGUAGE plpythonu
+ AS $$
+ val = [{'a': 1, 'b': 'boo', 'c': None}, {'d': 2}]
+ return val
+ $$;
+
+ SELECT test2arr();
+
+
+-- test as part of prepare/execute
+CREATE FUNCTION test3() RETURNS void
+LANGUAGE plpythonu
+AS $$
+rv = plpy.execute("SELECT 'aa=>bb, cc=>NULL'::hstore AS col1")
+plpy.info(repr(rv[0]["col1"]))
+
+val = {'a': 1, 'b': 'boo', 'c': None}
+plan = plpy.prepare("SELECT $1::text AS col1", ["hstore"])
+rv = plpy.execute(plan, [val])
+plpy.info(repr(rv[0]["col1"]))
+$$;
+
+SELECT test3();
+
+
+-- test inline
+DO LANGUAGE plpythonu $$
+rv = plpy.execute("SELECT 'aa=>bb, cc=>NULL'::hstore AS col1")
+plpy.info(repr(rv[0]["col1"]))
+
+val = {'a': 1, 'b': 'boo', 'c': None}
+plan = plpy.prepare("SELECT $1::text AS col1", ["hstore"])
+rv = plpy.execute(plan, [val])
+plpy.info(repr(rv[0]["col1"]))
+$$;
+
+
+-- test trigger
+CREATE TABLE test1 (a int, b hstore);
+INSERT INTO test1 VALUES (1, 'aa=>bb, cc=>NULL');
+SELECT * FROM test1;
+
+CREATE FUNCTION test4() RETURNS trigger
+LANGUAGE plpythonu
+AS $$
+plpy.info("Trigger row: %r" % TD["new"])
+if TD["new"]["a"] == 1:
+ TD["new"]["b"] = {'a': 1, 'b': 'boo', 'c': None}
+
+return "MODIFY"
+$$;
+
+CREATE TRIGGER test4 BEFORE UPDATE ON test1 FOR EACH ROW EXECUTE PROCEDURE test4();
+
+UPDATE test1 SET a = a;
+SELECT * FROM test1;
diff --git a/contrib/ltree_plpython/.gitignore b/contrib/ltree_plpython/.gitignore
new file mode 100644
index 0000000..5dcb3ff
--- /dev/null
+++ b/contrib/ltree_plpython/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/contrib/ltree_plpython/Makefile b/contrib/ltree_plpython/Makefile
new file mode 100644
index 0000000..34e03ce
--- /dev/null
+++ b/contrib/ltree_plpython/Makefile
@@ -0,0 +1,30 @@
+# contrib/ltree_plpython/Makefile
+
+MODULE_big = ltree_plpython$(python_majorversion)
+OBJS = ltree_plpython.o
+
+PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/ltree
+
+EXTENSION = ltree_plpythonu ltree_plpython2u ltree_plpython3u
+DATA = ltree_plpythonu--1.0.sql ltree_plpython2u--1.0.sql ltree_plpython3u--1.0.sql
+
+REGRESS = ltree_plpython
+REGRESS_PLPYTHON3_MANGLE := $(REGRESS)
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/ltree_plpython
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+REGRESS_OPTS = --extra-install=contrib/ltree --load-extension=ltree
+ifeq ($(python_majorversion),2)
+REGRESS_OPTS += --load-extension=plpythonu --load-extension=ltree_plpythonu
+endif
+
+include $(top_srcdir)/src/pl/plpython/regress-python3-mangle.mk
diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out
new file mode 100644
index 0000000..6626fe9
--- /dev/null
+++ b/contrib/ltree_plpython/expected/ltree_plpython.out
@@ -0,0 +1,42 @@
+CREATE EXTENSION plpython2u;
+CREATE EXTENSION ltree_plpython2u;
+CREATE FUNCTION test1(val ltree) RETURNS int
+LANGUAGE plpythonu
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+SELECT test1('aa.bb.cc'::ltree);
+INFO: ['aa', 'bb', 'cc']
+CONTEXT: PL/Python function "test1"
+ test1
+-------
+ 3
+(1 row)
+
+CREATE FUNCTION test1n(val ltree) RETURNS int
+LANGUAGE plpython2u
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+SELECT test1n('aa.bb.cc'::ltree);
+INFO: ['aa', 'bb', 'cc']
+CONTEXT: PL/Python function "test1n"
+ test1n
+--------
+ 3
+(1 row)
+
+CREATE FUNCTION test2() RETURNS ltree
+LANGUAGE plpythonu
+AS $$
+return ['foo', 'bar', 'baz']
+$$;
+-- plpython to ltree is not yet implemented, so this will fail,
+-- because it will try to parse the Python list as an ltree input
+-- string.
+SELECT test2();
+ERROR: syntax error at position 0
+CONTEXT: while creating return value
+PL/Python function "test2"
diff --git a/contrib/ltree_plpython/ltree_plpython.c b/contrib/ltree_plpython/ltree_plpython.c
new file mode 100644
index 0000000..111e3e3
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpython.c
@@ -0,0 +1,32 @@
+#include "postgres.h"
+#include "fmgr.h"
+#include "plpython.h"
+#include "ltree.h"
+
+PG_MODULE_MAGIC;
+
+
+PG_FUNCTION_INFO_V1(ltree_to_plpython);
+Datum ltree_to_plpython(PG_FUNCTION_ARGS);
+
+Datum
+ltree_to_plpython(PG_FUNCTION_ARGS)
+{
+ ltree *in = PG_GETARG_LTREE(0);
+ int i;
+ PyObject *list;
+ ltree_level *curlevel;
+
+ list = PyList_New(in->numlevel);
+
+ curlevel = LTREE_FIRST(in);
+ for (i = 0; i < in->numlevel; i++)
+ {
+ PyList_SetItem(list, i, PyString_FromStringAndSize(curlevel->name, curlevel->len));
+ curlevel = LEVEL_NEXT(curlevel);
+ }
+
+ PG_FREE_IF_COPY(in, 0);
+
+ return PointerGetDatum(list);
+}
diff --git a/contrib/ltree_plpython/ltree_plpython2u--1.0.sql b/contrib/ltree_plpython/ltree_plpython2u--1.0.sql
new file mode 100644
index 0000000..29a12d4
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpython2u--1.0.sql
@@ -0,0 +1,12 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpython2u;
+SELECT NULL::ltree;
+
+
+CREATE FUNCTION ltree_to_plpython2(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'ltree_to_plpython';
+
+CREATE TRANSFORM FOR ltree LANGUAGE plpython2u (
+ FROM SQL WITH FUNCTION ltree_to_plpython2(internal)
+);
diff --git a/contrib/ltree_plpython/ltree_plpython2u.control b/contrib/ltree_plpython/ltree_plpython2u.control
new file mode 100644
index 0000000..bedfd0a
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpython2u.control
@@ -0,0 +1,6 @@
+# ltree_plpython2u extension
+comment = 'transform between ltree and plpython2u'
+default_version = '1.0'
+module_pathname = '$libdir/ltree_plpython2'
+relocatable = true
+requires = 'ltree,plpython2u'
diff --git a/contrib/ltree_plpython/ltree_plpython3u--1.0.sql b/contrib/ltree_plpython/ltree_plpython3u--1.0.sql
new file mode 100644
index 0000000..1300a78
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpython3u--1.0.sql
@@ -0,0 +1,12 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpython3u;
+SELECT NULL::ltree;
+
+
+CREATE FUNCTION ltree_to_plpython3(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME', 'ltree_to_plpython';
+
+CREATE TRANSFORM FOR ltree LANGUAGE plpython3u (
+ FROM SQL WITH FUNCTION ltree_to_plpython3(internal)
+);
diff --git a/contrib/ltree_plpython/ltree_plpython3u.control b/contrib/ltree_plpython/ltree_plpython3u.control
new file mode 100644
index 0000000..96c9764
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpython3u.control
@@ -0,0 +1,6 @@
+# ltree_plpython3u extension
+comment = 'transform between ltree and plpython3u'
+default_version = '1.0'
+module_pathname = '$libdir/ltree_plpython3'
+relocatable = true
+requires = 'ltree,plpython3u'
diff --git a/contrib/ltree_plpython/ltree_plpythonu--1.0.sql b/contrib/ltree_plpython/ltree_plpythonu--1.0.sql
new file mode 100644
index 0000000..1d1af28
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpythonu--1.0.sql
@@ -0,0 +1,12 @@
+-- make sure the prerequisite libraries are loaded
+DO '1' LANGUAGE plpythonu;
+SELECT NULL::ltree;
+
+
+CREATE FUNCTION ltree_to_plpython(val internal) RETURNS internal
+LANGUAGE C STRICT IMMUTABLE
+AS 'MODULE_PATHNAME';
+
+CREATE TRANSFORM FOR ltree LANGUAGE plpythonu (
+ FROM SQL WITH FUNCTION ltree_to_plpython(internal)
+);
diff --git a/contrib/ltree_plpython/ltree_plpythonu.control b/contrib/ltree_plpython/ltree_plpythonu.control
new file mode 100644
index 0000000..b03c89a
--- /dev/null
+++ b/contrib/ltree_plpython/ltree_plpythonu.control
@@ -0,0 +1,6 @@
+# ltree_plpythonu extension
+comment = 'transform between ltree and plpythonu'
+default_version = '1.0'
+module_pathname = '$libdir/ltree_plpython2'
+relocatable = true
+requires = 'ltree,plpythonu'
diff --git a/contrib/ltree_plpython/sql/ltree_plpython.sql b/contrib/ltree_plpython/sql/ltree_plpython.sql
new file mode 100644
index 0000000..2785592
--- /dev/null
+++ b/contrib/ltree_plpython/sql/ltree_plpython.sql
@@ -0,0 +1,34 @@
+CREATE EXTENSION plpython2u;
+CREATE EXTENSION ltree_plpython2u;
+
+
+CREATE FUNCTION test1(val ltree) RETURNS int
+LANGUAGE plpythonu
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+
+SELECT test1('aa.bb.cc'::ltree);
+
+
+CREATE FUNCTION test1n(val ltree) RETURNS int
+LANGUAGE plpython2u
+AS $$
+plpy.info(repr(val))
+return len(val)
+$$;
+
+SELECT test1n('aa.bb.cc'::ltree);
+
+
+CREATE FUNCTION test2() RETURNS ltree
+LANGUAGE plpythonu
+AS $$
+return ['foo', 'bar', 'baz']
+$$;
+
+-- plpython to ltree is not yet implemented, so this will fail,
+-- because it will try to parse the Python list as an ltree input
+-- string.
+SELECT test2();
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index e638a8f..c4b3b42 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -264,6 +264,11 @@ System Catalogs
+ pg_transform
+ transforms (data type to procedural language conversions)
+
+
+
pg_trigger
triggers
@@ -5722,6 +5727,74 @@ <structname>pg_tablespace</> Columns
+
+ <structname>pg_transform</structname>
+
+
+ pg_transform
+
+
+
+ The catalog pg_transform stores information about
+ transforms, which are a mechanism to adapt data types to procedural
+ languages. See for more information.
+
+
+ Re: [PATCH] Add transforms feature
От:
Cédric Villemain <cedric@2ndquadrant.com>
Дата:
Peter Eisentraut a écrit : >A transform is an SQL object that supplies to functions for converting >between data types and procedural languages. For example, a transform >could arrange that hstore is converted to an appropriate hash or >dictionary object in PL/Perl or PL/Python. Nice ! >Continued from 2013-01 commit fest. All known open issues have been >fixed. You kept PGXS style makefile... -- Envoyé de mon téléphone excusez la brièveté.
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 6/14/13 3:46 AM, Cédric Villemain wrote: > You kept PGXS style makefile... I know, but that's a separate issue that hasn't been decided yet.
Re: [PATCH] Add transforms feature
От:
Craig Ringer <craig@2ndquadrant.com>
Дата:
On 06/14/2013 11:11 AM, Peter Eisentraut wrote: > A transform is an SQL object that supplies to functions for converting > between data types and procedural languages. For example, a transform > could arrange that hstore is converted to an appropriate hash or > dictionary object in PL/Perl or PL/Python. > > Externally visible changes: > > - new SQL commands CREATE TRANSFORM and DROP TRANSFORM > > - system catalog pg_transform > > - transform support in PL/Perl and PL/Python > > - PL/Perl and PL/Python install their header files for use by external > types I wonder if that should be extended to install headers for hstore, ltree, and while we're at it, intarray as well? You handle getting access to the headers by using include path flags: PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/hstore which is fine for in-tree builds but AFAIK means that pgxs cannot build these extensions. If the extension modules installed their headers that problem would go away; you'd just #include "contrib/hstore/hstore.h" Many modules already have useful non-static functions that work with raw types and aren't callable via the fmgr, as well as the macros and typdefs that make working with them easier. intarray in particular would be extremely useful to use from other extensions, but in the past I've landed up copying it and then adding the extra functions I needed because I couldn't access its header in a PGXS build. Peter's already answered my main worry on this, which is whether any platform would prevent us from doing the required runtime symbol lookup from shared libs loaded at the same level. I knew RTLD_GLOBAL allowed it for dlopen(...), but wasn't sure about other platforms. Thoughts? Should contrib modules be able to install headers and use each others' symbols? I've seen interest in this in the wild from people who want to use hstore in their own extensions and I've had uses for it myself with intarray. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 6/14/13 11:48 PM, Craig Ringer wrote: > I wonder if that should be extended to install headers for hstore, > ltree, and while we're at it, intarray as well? Sure, if someone wants to go through and check which headers are independently usable, and do the necessarily cleanups with necessary.
Re: [PATCH] Add transforms feature
От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
Peter Eisentraut wrote: > A transform is an SQL object that supplies to functions for converting > between data types and procedural languages. For example, a transform > could arrange that hstore is converted to an appropriate hash or > dictionary object in PL/Perl or PL/Python. > > Externally visible changes: This is a large patch. Do you intend to push the whole thing as a single commit, or split it? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [PATCH] Add transforms feature
От:
Craig Ringer <craig@2ndquadrant.com>
Дата:
On 06/18/2013 04:58 AM, Peter Eisentraut wrote: > On 6/14/13 11:48 PM, Craig Ringer wrote: >> I wonder if that should be extended to install headers for hstore, >> ltree, and while we're at it, intarray as well? > Sure, if someone wants to go through and check which headers are > independently usable, and do the necessarily cleanups with necessary. I can do that if there are no objections. It's only tangental to this work really, so I'll post a separate thread when I get on to it. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 6/17/13 5:31 PM, Alvaro Herrera wrote: > This is a large patch. Do you intend to push the whole thing as a > single commit, or split it? I thought about splitting it up, but I didn't find a reasonable way to do it.
Re: [PATCH] Add transforms feature
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, I've been playing with the new patch, and haven't been able to reproduce the load problem created by the original patch. So that seems fixed. I'm not comfortable with having all of the transform mappings in the main contrib/ directory though. Can we add a subdirectory called "transforms" containing all of these? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: [PATCH] Add transforms feature
От:
Hitoshi Harada <umi.tanuki@gmail.com>
Дата:
On Thu, Jun 13, 2013 at 8:11 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
The patch applies and regression including contrib passes in my Mac. I know I came late, but I have a few questions.
- vs SQL standard
- dependency loading issue
- function types
Hitoshi Harada
A transform is an SQL object that supplies to functions for converting
between data types and procedural languages. For example, a transform
could arrange that hstore is converted to an appropriate hash or
dictionary object in PL/Perl or PL/Python.
The patch applies and regression including contrib passes in my Mac. I know I came late, but I have a few questions.
- vs SQL standard
I'm worried about overloading the standard. As document says, SQL standard defines CREATE TRANSFORM syntax which is exactly the same as this proposal but for different purpose. The standard feature is the data conversion between client and server side data type, I guess. I am concerned about it because in the future if someone wants to implement this SQL standard feature, there is no way to break thing. I'd be happy if subsequent clause was different. CREATE TYPE has two different syntax, one for composite type and one for internal user-defined type (I'm not sure either is defined in the standard, though) and I think we could do something like that. Or as someone suggested in the previous thread, it might be a variant of CAST. CREATE CAST (hstore AS plpython2u) ? Or CREATE LANGUAGE TRANSFORM might sound better. In either case, I think we are missing the discussion on the standard overloading.
Although most of the use cases are via CREATE EXTENSION, it is not great to let users to load the dependency manually. Is it possible to load hstore.so and plpython2u.so from _PG_init of hstore_plpython2u? Because the author of transform should certainly know the name of shared library in the database installation, writing down the shared library names in the init function sounds reasonable. Or do we still need to consider cases where plpython2u.so is renamed to something else?
Although I read the suggestion to use internal type as the argument of from_sql function, I guess it exposes another security risk. Since we don't know what SQL type can safely be passed to the from_sql function, we cannot check if the function is the right one at the time of CREATE TRANSFORM. Non-super user can add his user defined type and own it, and create a transform that with from_sql function that takes internal and crashes with this user-defined type. A possible compromise is let only super user create transforms, or come up with nice way to allow func(sql_type) -> internal signature.
- create or replace causes inconsistencyI tried:
* create transform python to hstore (one way transform)
* create function f(h hstore) language python
* create or replace transform hstore to python and python to hstore (both ways)
* call f() causes error, since it misses hstore to python transform. It is probably looking at the old definition
- create func -> create transform is not prohibited I saw your post in the previous discussion:
> > * I don't think recording dependencies on transforms used when creating
> > functions is a good idea as the transform might get created after the
> > functions already exists. That seems to be a pretty confusing behaviour.
>
> We need the dependencies, because otherwise dropping a transform would
> break or silently alter the behavior of functions that depend on it.
> That sounds like my worst nightmare, thinking of some applications that
> would be affected by that. But your point is a good one. I think this
> could be addressed by prohibiting the creation of a transform that
> affects functions that already exist.
> > * I don't think recording dependencies on transforms used when creating
> > functions is a good idea as the transform might get created after the
> > functions already exists. That seems to be a pretty confusing behaviour.
>
> We need the dependencies, because otherwise dropping a transform would
> break or silently alter the behavior of functions that depend on it.
> That sounds like my worst nightmare, thinking of some applications that
> would be affected by that. But your point is a good one. I think this
> could be addressed by prohibiting the creation of a transform that
> affects functions that already exist.
However I don't see this prohibition of create transform if there is already such function. You are not planning to address this issue?
For now, that's it. I'm going to dig more later.
Thanks,
Hitoshi Harada
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 7/3/13 7:15 PM, Josh Berkus wrote: > I'm not comfortable with having all of the transform mappings in the > main contrib/ directory though. Can we add a subdirectory called > "transforms" containing all of these? I don't see any value in that. The data types they apply to are in contrib after all.
Re: [PATCH] Add transforms feature
От:
Hitoshi Harada <umi.tanuki@gmail.com>
Дата:
On Friday, July 5, 2013, Peter Eisentraut wrote:
On 7/3/13 7:15 PM, Josh Berkus wrote:
> I'm not comfortable with having all of the transform mappings in the
> main contrib/ directory though. Can we add a subdirectory called
> "transforms" containing all of these?
I don't see any value in that. The data types they apply to are in
contrib after all.
I guess his suggestion is contrib/transforms directory, not transforms directory at top level. Stil you don't see value?
--
Hitoshi Harada
Re: [PATCH] Add transforms feature
От:
Josh Berkus <josh@agliodbs.com>
Дата:
On 07/05/2013 09:08 AM, Peter Eisentraut wrote: > On 7/3/13 7:15 PM, Josh Berkus wrote: >> I'm not comfortable with having all of the transform mappings in the >> main contrib/ directory though. Can we add a subdirectory called >> "transforms" containing all of these? > > I don't see any value in that. The data types they apply to are in > contrib after all. Well, I had three thoughts on this: (a) transforms aren't like other contribs, in that they are dependant on other contribs before you install them. (b) we can expect maybe a dozen to 18 of them in core based on the data types there, and I hate to clutter up /contrib, and (c) I'd like to do a future feature which supports "install all transforms" functionality, which would be helped by having them in their own directory. That's my thinking on it anyway. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: [PATCH] Add transforms feature
От:
Dimitri Fontaine <dimitri@2ndQuadrant.fr>
Дата:
Josh Berkus writes: > (c) I'd like to do a future feature which supports "install all > transforms" functionality, which would be helped by having them in their > own directory. I think we should install required extensions automatically when they are available. Also, we will need better tooling to deal with extension dependencies, see my patch for that from some time ago. https://commitfest.postgresql.org/action/patch_view?id=727 Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Thu, 2013-07-04 at 02:18 -0700, Hitoshi Harada wrote: > as someone suggested in the previous thread, it might be a variant of > CAST. CREATE CAST (hstore AS plpython2u) ? Or CREATE LANGUAGE TRANSFORM > might sound better. In either case, I think we are missing the discussion > on the standard overloading. LANGUAGE isn't a concept limited to the server side in the SQL standard. I could go with something like CREATE SERVER TRANSFORM. > - dependency loading issue > Although most of the use cases are via CREATE EXTENSION, it is not great to > let users to load the dependency manually. Is it possible to load > hstore.so and plpython2u.so from _PG_init of hstore_plpython2u? Because > the author of transform should certainly know the name of shared library in > the database installation, writing down the shared library names in the > init function sounds reasonable. Or do we still need to consider cases > where plpython2u.so is renamed to something else? I don't like my solution very much either, but I think I like this one even less. I think the identity of the shared library for the hstore type is the business of the hstore extension, and other extensions shouldn't mess with it. The interfaces exposed by the hstore extension are the types and functions, and that's what we are allowed to use. If that's not enough, we need to expose more interfaces. > - function types > Although I read the suggestion to use internal type as the argument of > from_sql function, I guess it exposes another security risk. Since we > don't know what SQL type can safely be passed to the from_sql function, we > cannot check if the function is the right one at the time of CREATE > TRANSFORM. Non-super user can add his user defined type and own it, and > create a transform that with from_sql function that takes internal and > crashes with this user-defined type. A possible compromise is let only > super user create transforms, or come up with nice way to allow > func(sql_type) -> internal signature. Good point. My original patch allowed func(sql_type) -> internal, but I took that out because people had security concerns. I'd be OK with restricting transform creation to superusers in the first cut. > - create or replace causes inconsistency > I tried: > * create transform python to hstore (one way transform) > * create function f(h hstore) language python > * create or replace transform hstore to python and python to hstore (both > ways) > * call f() causes error, since it misses hstore to python transform. It > is probably looking at the old definition What error exactly? Can you show the full test case? There might be some caching going on. > - create func -> create transform is not prohibited > I saw your post in the previous discussion: > > > > * I don't think recording dependencies on transforms used when creating > > > functions is a good idea as the transform might get created after the > > > functions already exists. That seems to be a pretty confusing behaviour. > > > > We need the dependencies, because otherwise dropping a transform would > > break or silently alter the behavior of functions that depend on it. > > That sounds like my worst nightmare, thinking of some applications that > > would be affected by that. But your point is a good one. I think this > > could be addressed by prohibiting the creation of a transform that > > affects functions that already exist. > > However I don't see this prohibition of create transform if there is > already such function. You are not planning to address this issue? I had planned to implement that, but talking to some people most didn't think it was useful or desirable. It's still open for debate.
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Fri, 2013-07-05 at 12:04 -0700, Josh Berkus wrote: > (a) transforms aren't like other contribs, in that they are dependant on > other contribs before you install them. That doesn't appear to be a reason for creating subdirectories. > (b) we can expect maybe a dozen to 18 of them in core based on the data > types there, and I hate to clutter up /contrib, and Well, that's a matter of opinion. I'd be more happy with 250 contribs all on the same level versus a bunch of subdirectories structured based on personal preferences. But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) > (c) I'd like to do a future feature which supports "install all > transforms" functionality, which would be helped by having them in their > own directory. Installing all transforms by itself is not a sensible operation, because you only want the transforms for the types and languages that you actually use or have previously selected for installation. I understand that this situation is unsatisfactory. But I don't think any dependency or package management system has solved it. For example, can any package management system make the following decision: user install PHP, user installs PostgreSQL client => install php-pgsql automatically. I don't think so. The only solutions are making PHP dependent on PostgreSQL (or vice versa), or having the user install php-pgsql explicitly.
Re: [PATCH] Add transforms feature
От:
Josh Berkus <josh@agliodbs.com>
Дата:
Peter, On 07/07/2013 12:06 PM, Peter Eisentraut wrote: > Good point. My original patch allowed func(sql_type) -> internal, but I > took that out because people had security concerns. > > I'd be OK with restricting transform creation to superusers in the first > cut. Have we added the ability of non-superusers to create extensions yet? Until we have that, there's no point in allowing them to load transforms. Once we have that, we'll need to let them do it. >>> We need the dependencies, because otherwise dropping a transform would >>> break or silently alter the behavior of functions that depend on it. >>> That sounds like my worst nightmare, thinking of some applications that >>> would be affected by that. But your point is a good one. I think this >>> could be addressed by prohibiting the creation of a transform that >>> affects functions that already exist. >> >> However I don't see this prohibition of create transform if there is >> already such function. You are not planning to address this issue? > > I had planned to implement that, but talking to some people most didn't > think it was useful or desirable. It's still open for debate. I don't think it's desirable. It would be hard to do, and at some level we need to make a presumption of competence on the part of the DBA. We should put a warning in the docs, though. >> (b) we can expect maybe a dozen to 18 of them in core based on the data >> types there, and I hate to clutter up /contrib, and > > Well, that's a matter of opinion. I'd be more happy with 250 contribs > all on the same level versus a bunch of subdirectories structured based > on personal preferences. > > But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) Yeah, I'd like to see some other opinions on this. >> (c) I'd like to do a future feature which supports "install all >> transforms" functionality, which would be helped by having them in their >> own directory. > > Installing all transforms by itself is not a sensible operation, because > you only want the transforms for the types and languages that you > actually use or have previously selected for installation. Give me some credit. I'm talking about a script for "install all transforms for which the dependancies are already installed". That's certainly entirely doable, and would be made easier by putting the transforms in their own directory or otherwise flagging them to identify them as transforms. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: [PATCH] Add transforms feature
От:
Hitoshi Harada <umi.tanuki@gmail.com>
Дата:
On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut wrote:
> On Thu, 2013-07-04 at 02:18 -0700, Hitoshi Harada wrote:
>> as someone suggested in the previous thread, it might be a variant of
>> CAST. CREATE CAST (hstore AS plpython2u) ? Or CREATE LANGUAGE TRANSFORM
>> might sound better. In either case, I think we are missing the discussion
>> on the standard overloading.
>
> LANGUAGE isn't a concept limited to the server side in the SQL standard.
> I could go with something like CREATE SERVER TRANSFORM.
I like it better than the current one.
>> - dependency loading issue
>> Although most of the use cases are via CREATE EXTENSION, it is not great to
>> let users to load the dependency manually. Is it possible to load
>> hstore.so and plpython2u.so from _PG_init of hstore_plpython2u? Because
>> the author of transform should certainly know the name of shared library in
>> the database installation, writing down the shared library names in the
>> init function sounds reasonable. Or do we still need to consider cases
>> where plpython2u.so is renamed to something else?
>
> I don't like my solution very much either, but I think I like this one
> even less. I think the identity of the shared library for the hstore
> type is the business of the hstore extension, and other extensions
> shouldn't mess with it. The interfaces exposed by the hstore extension
> are the types and functions, and that's what we are allowed to use. If
> that's not enough, we need to expose more interfaces.
OK, my idea was worse, because the symbol resolution happens before
_PG_init anyway. But what I feel is, why can't we load dependency
libraries automatically? plpython can load libpython automatically, I
guess. Is it only the matter of LD_LIBRARY_PATH stuff?
>> - function types
>> Although I read the suggestion to use internal type as the argument of
>> from_sql function, I guess it exposes another security risk. Since we
>> don't know what SQL type can safely be passed to the from_sql function, we
>> cannot check if the function is the right one at the time of CREATE
>> TRANSFORM. Non-super user can add his user defined type and own it, and
>> create a transform that with from_sql function that takes internal and
>> crashes with this user-defined type. A possible compromise is let only
>> super user create transforms, or come up with nice way to allow
>> func(sql_type) -> internal signature.
>
> Good point. My original patch allowed func(sql_type) -> internal, but I
> took that out because people had security concerns.
>
> I'd be OK with restricting transform creation to superusers in the first
> cut.
Yeah, I think it's better to limit to superuser.
>> - create or replace causes inconsistency
>> I tried:
>> * create transform python to hstore (one way transform)
>> * create function f(h hstore) language python
>> * create or replace transform hstore to python and python to hstore (both
>> ways)
>> * call f() causes error, since it misses hstore to python transform. It
>> is probably looking at the old definition
>
> What error exactly? Can you show the full test case?
>
> There might be some caching going on.
Here is the full set of what's happening.
test=# create extension hstore;
CREATE EXTENSION
test=# create extension plpython2u;
CREATE EXTENSION
test=# CREATE FUNCTION hstore_to_plpython2(val internal) RETURNS internal
test-# LANGUAGE C STRICT IMMUTABLE
test-# AS '$libdir/hstore_plpython2', 'hstore_to_plpython';
CREATE FUNCTION
test=#
test=# CREATE FUNCTION plpython2_to_hstore(val internal) RETURNS hstore
test-# LANGUAGE C STRICT IMMUTABLE
test-# AS 'hstore_plpython2', 'plpython_to_hstore';
CREATE FUNCTION
test=#
test=# CREATE TRANSFORM FOR hstore LANGUAGE plpython2u (
test(# FROM SQL WITH FUNCTION hstore_to_plpython2(internal),
test(# TO SQL WITH FUNCTION plpython2_to_hstore(internal)
test(# );
CREATE TRANSFORM
test=# create or replace function f(h hstore) returns hstore as $$
test$# h['b'] = 10
test$# return h
test$# $$ language plpython2u;
CREATE FUNCTION
test=# select f('a=>1'); f
---------------------"a"=>"1", "b"=>"10"
(1 row)
test=# CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plpython2u (
test(# TO SQL WITH FUNCTION plpython2_to_hstore(internal)
test(# );
CREATE TRANSFORM
test=# select f('a=>1'); f
---------------------"a"=>"1", "b"=>"10"
(1 row)
test=# \c
You are now connected to database "test" as user "haradh1".
test=# select f('a=>1');
ERROR: TypeError: 'str' object does not support item assignment
CONTEXT: Traceback (most recent call last): PL/Python function "f", line 2, in h['b'] = 10
PL/Python function "f"
After reconnecting to the database with \c, the function behavior
changed because we did CREATE OR REPLACE. If we go with dependency
between function and transform, we shouldn't support OR REPLACE, I
guess. plpython can throw away the cache, but I feel like not
supporting OR REPLACE in this case.
--
Hitoshi Harada
Re: [PATCH] Add transforms feature
От:
Hitoshi Harada <umi.tanuki@gmail.com>
Дата:
On Thu, Jul 4, 2013 at 2:18 AM, Hitoshi Harada wrote: > For now, that's it. I'm going to dig more later. > After looking into rest of the change, - TYPTYPE_DOMAIN is not supported. Why did you specifically disallow it? - ParseFuncOrColumn now prohibits to find function returning internal, but is it effective? I think it's already disallowed, or there is no way to call it. - get_transform_oid and get_transform are redundant -- Hitoshi Harada
Re: [PATCH] Add transforms feature
От:
Josh Berkus <josh@agliodbs.com>
Дата:
On 07/08/2013 12:00 PM, Josh Berkus wrote: >>> (b) we can expect maybe a dozen to 18 of them in core based on the data >>> >> types there, and I hate to clutter up /contrib, and >> > >> > Well, that's a matter of opinion. I'd be more happy with 250 contribs >> > all on the same level versus a bunch of subdirectories structured based >> > on personal preferences. >> > >> > But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) > Yeah, I'd like to see some other opinions on this. > Well, based on the total lack of opinions from anyone on -hackers, I'd say we leave your patch alone and leave the transforms in their current directory location. We can always move them later. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
CREATE TRANSFORM syntax (was Re: [PATCH] Add transforms feature)
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Mon, 2013-07-08 at 23:00 -0700, Hitoshi Harada wrote: > On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut > wrote: > > On Thu, 2013-07-04 at 02:18 -0700, Hitoshi Harada wrote: > >> as someone suggested in the previous thread, it might be a variant > of > >> CAST. CREATE CAST (hstore AS plpython2u) ? Or CREATE LANGUAGE > TRANSFORM > >> might sound better. In either case, I think we are missing the > discussion > >> on the standard overloading. > > > > LANGUAGE isn't a concept limited to the server side in the SQL > standard. > > I could go with something like CREATE SERVER TRANSFORM. > > I like it better than the current one. I had started to work on making this adjustment, but found the result very ugly. It also created a confusing association with CREATE SERVER, which is something different altogether. My next best idea is CREATE TRANSFORM FOR hstore SERVER LANGUAGE plperl, which preserves the overall idea but still distinguishes server from client languages. Comments?
Re: CREATE TRANSFORM syntax (was Re: [PATCH] Add transforms feature)
От:
Josh Berkus <josh@agliodbs.com>
Дата:
On 08/13/2013 07:16 PM, Peter Eisentraut wrote: > My next best idea is CREATE TRANSFORM FOR hstore SERVER LANGUAGE plperl, > which preserves the overall idea but still distinguishes server from > client languages. > > Comments? My thinking is that TRANSFORMS will almost certainly be managed by installer/puppet scripts by users, so it doesn't really matter how ugly the syntax is, as long as it's unambiguous. Which is a roundabout way of saying "whatever syntax you implement is fine with me from a usability perspective". -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: [PATCH] Add transforms feature
Here is a new patch which addresses some of the issues you found. > - vs SQL standard > After reviewing this, there is actually no conflict with the SQL standard, because the CREATE TRANSFORM syntax in the SQL standard uses an additional transform group clause which I don't use. So there is no ambiguity. Hence, I left the syntax unchanged. > - function types I changed this so that CREATE TRANSFORM requires owning the from-SQL and to-SQL functions. > - create or replace causes inconsistency > Fixed this by adding cache invalidation in PL/Python.
Re: [PATCH] Add transforms feature
Rebased patch. No changes except that merge conflicts were resolved, and I had to add some Data::Dumper tweaks to the regression tests so that the results came out in consistent order on different versions of Perl.
Re: [PATCH] Add transforms feature
От:
Dimitri Fontaine <dimitri@2ndQuadrant.fr>
Дата:
Hi, Peter Eisentraut writes: > Rebased patch. No changes except that merge conflicts were resolved, > and I had to add some Data::Dumper tweaks to the regression tests so > that the results came out in consistent order on different versions of > Perl. I just spent some time reading that patch, here's my first round of review: - Documentation style seems to be to be different from the "man page" or "reference docs" style that we use elsewhere, and is instead deriving the general case from examples. Reads strange. - The internal datatype argument and return type discussion for function argument looks misplaced, but I don't have a better proposition for that. - The code looks good, I didn't spot any problem when reading it. Given your Jenkins installation I didn't try (yet at least) to use the patch and compile and run it locally. Interesting note might be the addition of two new system caches, one for the PL languages and another one for the transform functions. I agree with the need, just wanted to raise awereness about that in case there's something to be said on that front. - Do we need an ALTER TRANSFORM command? Usually we have at least an Owner for the new objects and a command to change the owner. Then should we be able to change the function(s) used in a transform? - Should transform live in a schema? At first sight, no reason why, but see next point about a use case that we might be able to solve doing that. - SQL Standard has something different named the same thing, targetting client side types apparently. Is there any reason why we would want to stay away from using the same name for something really different in PostgreSQL? On the higher level design, the big question here is about selective behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl then any plperl function will now receive its hstore arguments as a proper perl hash rather than a string. Any pre-existing plperl function with hstore arguments or return type then needs to be upgraded to handle the new types nicely, and some of those might not be under the direct control of the DBA running the CREATE TRANSFORM command, when using some plperl extensions for example. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. We could design transforms to only "activate" in the schema they are created, thus allowing plperl functions to co-exist where some will receive proper hash for hstores and other will continue to get plain text. Should using the schema to that ends be frowned upon, then we need a way to register each plperl function against using or not using the transform facility, defaulting to not using anything. Maybe something like the following: CREATE FUNCTION foo(hash hstore, x ltree) RETURNS hstore LANGUAGE plperl USING TRANSFORM FOR hstore, ltree AS $$ … $$; Worst case, that I really don't think we need, would be addressing that per-argument: CREATE FUNCTION foo (hash hstore WITH TRANSFORM, kv hstore) … I certainly hope we don't need that, and sure can't imagine use cases for that level of complexity at the time of writing this review. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 11/15/13, 11:04 AM, Dimitri Fontaine wrote: > - Documentation style seems to be to be different from the "man page" > or "reference docs" style that we use elsewhere, and is instead > deriving the general case from examples. Reads strange. Which specific section do you have in mind? It's hard to explain this feature in abstract terms, I think. > - The internal datatype argument and return type discussion for > function argument looks misplaced, but I don't have a better > proposition for that. OK, maybe I'll put that in parentheses or a separate paragraph. > - Do we need an ALTER TRANSFORM command? > > Usually we have at least an Owner for the new objects and a command > to change the owner. Then should we be able to change the > function(s) used in a transform? We don't have ALTER CAST either, and no one's been too bothered about that. It's possible, of course. > - Should transform live in a schema? > > At first sight, no reason why, but see next point about a use case > that we might be able to solve doing that. Transforms don't have a name, so I don't quite see what you mean here. > - SQL Standard has something different named the same thing, > targetting client side types apparently. Is there any reason why we > would want to stay away from using the same name for something > really different in PostgreSQL? Let's review that, as there as been some confusion about that. The SQL standard syntax is CREATE TRANSFORM FOR (...details...); and then there is SET DEFAULT TRANSFORM GROUP SET TRANSFORM GROUP FOR TYPE This is essentially an elaborate way to have custom input/output formats, like DateStyle or bytea_output. (You can find examples of this in the IBM DB2 documentation. Some of their clients apparently set a certain transform group automatically, allowing you to set per-interface output formats.) The proposed syntax in the other hand is CREATE TRANSFORM FOR LANGUAGE (...details...); So you could consider LANGUAGE to be the implicit transform group of language , if you like. Or you could consider that this is a situation like VIEW vs. MATERERIALIZED VIEW: they sound the same, they are a bit alike, but the implementation details are different. All obvious synonyms of "transform" (conversion, translation, etc.) are already in use. > On the higher level design, the big question here is about selective > behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl > then any plperl function will now receive its hstore arguments as a > proper perl hash rather than a string. > > Any pre-existing plperl function with hstore arguments or return type > then needs to be upgraded to handle the new types nicely, and some of > those might not be under the direct control of the DBA running the > CREATE TRANSFORM command, when using some plperl extensions for example. I had proposed disallowing installing a transform that would affect existing functions. That was rejected or deemed unnecessary. You can't have it both ways. ;-) > A mechanism allowing for the transform to only be used in some functions > but not others might be useful. The simplest such mechanism I can think > of is modeled against the PL/Java classpath facility as specified in the > SQL standard: you attach a classpath per schema. Anything that's a problem per-database would also be a problem per schema. > Should using the schema to that ends be frowned upon, then we need a way > to register each plperl function against using or not using the > transform facility, defaulting to not using anything. Maybe something > like the following: > > CREATE FUNCTION foo(hash hstore, x ltree) > RETURNS hstore > LANGUAGE plperl > USING TRANSFORM FOR hstore, ltree > AS $$ … $$; This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. How many people actually use hstore with PL/Perl or PL/Python now? Probably not many, because it's weird. I like to think about how this works for new development: Here is my extension type, here is how it interfaces with languages. Once you have established that, you don't want to have to repeat that every time you write a function. That's error prone and cumbersome. And anything that's set per schema or higher is a dependency tracking and caching mess. Also, extension types should work the same as built-in types. Eventually, I'd like to rip out the hard-coded data type support in PL/Python and replace it with built-in transforms. Even if we don't actually do it, conceptually it should be possible. Now if we require "USING TRANSFORM FOR int, bytea" every time, we'd have taken a big step back. Effectively, we already have built-in transforms in PL/Python. We have added a few more over the years. It's been a bit of a pain from time to time. At least, with this feature we'd be moving this decision into user space and give people a way to fix things. (Incidentally, if you add a lot of transforms, you are probably dealing with a strongly typed language. And a strongly typed language is more likely to cleanly catch type errors resulting from changes in the transforms.)
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut wrote: > This is a transition problem. Nobody is required to install the > transforms into their existing databases. They probably shouldn't. Sure, but that's like saying "nobody's required to use this behavior-changing GUC, so it's OK to have a behavior-changing GUC". The point I think Dimitri is making, which IMHO is entirely valid, is that the feature as currently designed is database-wide. You either get this behavior for all of your functions, or you get it for none of them, and that might well not be what you want. For example, it's easy to imagine that you might want to install extensions A and B. A expects that a certain transform is loaded into the database, and B expects that it isn't. You now have created a situation where extensions A and B can't be used together. That sucks. If the transform were a property of particular function argument positions, this wouldn't be a problem. You could declare, in effect, that a certain function takes a transformed hstore, and this other one takes a non-transformed hstore. Now life is good. But that's not what is being proposed. You could argue that such a level of granularity is overkill, but frankly I've never had a real good feeling about the likelihood of these transforms getting installed in the first place. In theory, if you're using hstore and you're using plperl, you ought to also install hstore-plperl-transform, but I bet a significant percentage of people won't. So I don't foresee a finite transition period after which databases without transforms go away and all code is written with the assumption that transforms are available; instead, I foresee different people assuming different things and ending up with mutually incompatible code bases. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
От:
Dimitri Fontaine <dimitri@2ndQuadrant.fr>
Дата:
Hi, Allow me to temporarily skip important questions that you asked so that we can focus on the main problem here. As soon as we decide how to handle any kind of selectivity for the transforms, then I'm back to answering the other things. Peter Eisentraut writes: > Let's review that, as there as been some confusion about that. The SQL > standard syntax is Thanks for that. I see that you reused parts of the concept and keywords and implemented something specific to PostgreSQL from there. As there's no collision within the command sets, I think that problem is cleared. > I had proposed disallowing installing a transform that would affect > existing functions. That was rejected or deemed unnecessary. You can't > have it both ways. ;-) Well I'm not so sure, that's the point. >> A mechanism allowing for the transform to only be used in some functions >> but not others might be useful. The simplest such mechanism I can think >> of is modeled against the PL/Java classpath facility as specified in the >> SQL standard: you attach a classpath per schema. > > Anything that's a problem per-database would also be a problem per > schema. But a smaller problem, and as problem get smaller they get easier to reason about and fix too. In the example given by Robert Haas in the same thread, you could install extension A in schema A using the transforms for hstore and plperl and extension B in schema B not using the same transforms. I'm not saying using the schema that way is awesome, just that we have solid precedent in the standard and in pljava, and it looks easy enough to implement (we already have search_path invalidations IIRC). > This is a transition problem. Nobody is required to install the > transforms into their existing databases. They probably shouldn't. I disagree. > How many people actually use hstore with PL/Perl or PL/Python now? > Probably not many, because it's weird. > > I like to think about how this works for new development: Here is my > extension type, here is how it interfaces with languages. Once you have > established that, you don't want to have to repeat that every time you > write a function. That's error prone and cumbersome. And anything > that's set per schema or higher is a dependency tracking and caching mess. The problem is installing a set of extensions where some of them are already using the new transform feature and some of them are not. We need a way to cater with that, I think. > Also, extension types should work the same as built-in types. > Eventually, I'd like to rip out the hard-coded data type support in > PL/Python and replace it with built-in transforms. Even if we don't > actually do it, conceptually it should be possible. Now if we require I like that idea a lot. I don't see how you can make it work by default, as like Robert I think the transition phase you're talking about will never end. > "USING TRANSFORM FOR int, bytea" every time, we'd have taken a big step > back. Effectively, we already have built-in transforms in PL/Python. For core types only. > We have added a few more over the years. It's been a bit of a pain from > time to time. At least, with this feature we'd be moving this decision > into user space and give people a way to fix things. (Incidentally, if > you add a lot of transforms, you are probably dealing with a strongly > typed language. And a strongly typed language is more likely to cleanly > catch type errors resulting from changes in the transforms.) I think use space will want to be able to use code written using different sets of transforms for the same set of types, rather than being forced into upgrading their whole code at once. It reminds me of the python 2 to python 3 upgrade path. This is not solved yet, with libs that are python2 only and others that are python3 only, and OS that would ship some but not others. We already have the problem that shipping contrib by default is frowned upon at some places, then they miss out of plenty of awesome extensions. Transforms with no granularity is only going to make it worse, I think. So we have to choose the granularity: - per database (current patch), - per schema (standard has precedents with pljava, - per extension (forcing users into using extensions, not good), - per function (hard to maintain), - something else. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: [PATCH] Add transforms feature
От:
Hannu Krosing <hannu@2ndQuadrant.com>
Дата:
On 11/12/2013 12:21 PM, Peter Eisentraut wrote: > A transform is an SQL object that supplies to functions for converting > between data types and procedural languages. How hard would it be to extend this to add transforms directly between pairs of procedural languages ? One example would be calling a pl/v8 function from pl/python and converting directly between integers in both, without going through PostgreSQL type. Another and maybe even more interesting would be automatic null-transforms between two pl/python functions. Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [PATCH] Add transforms feature
От:
Hannu Krosing <hannu@2ndQuadrant.com>
Дата:
On 11/20/2013 10:58 PM, Robert Haas wrote: > On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut wrote: >> This is a transition problem. Nobody is required to install the >> transforms into their existing databases. They probably shouldn't. > Sure, but that's like saying "nobody's required to use this > behavior-changing GUC, so it's OK to have a behavior-changing GUC". > > The point I think Dimitri is making, which IMHO is entirely valid, is > that the feature as currently designed is database-wide. You either > get this behavior for all of your functions, or you get it for none of > them, and that might well not be what you want. For example, it's > easy to imagine that you might want to install extensions A and B. A > expects that a certain transform is loaded into the database, and B > expects that it isn't. You now have created a situation where > extensions A and B can't be used together. That sucks. > > If the transform were a property of particular function argument > positions, this wouldn't be a problem. You could declare, in effect, > that a certain function takes a transformed hstore, and this other one > takes a non-transformed hstore. Now life is good. But that's not > what is being proposed. You mean something like CREATE FUNCTION f(i int, h1 hstore USING TRANSFORM x, h2 hstore) ... where h1 would go through transform x and 1 and h2 would use "default transform" ? Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [PATCH] Add transforms feature
От:
Hannu Krosing <hannu@2ndQuadrant.com>
Дата:
On 11/15/2013 05:04 PM, Dimitri Fontaine wrote: > Hi, > > Peter Eisentraut writes: >> Rebased patch. No changes except that merge conflicts were resolved, >> and I had to add some Data::Dumper tweaks to the regression tests so >> that the results came out in consistent order on different versions of >> Perl. .... > On the higher level design, the big question here is about selective > behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl > then any plperl function will now receive its hstore arguments as a > proper perl hash rather than a string. > > Any pre-existing plperl function with hstore arguments or return type > then needs to be upgraded to handle the new types nicely, and some of > those might not be under the direct control of the DBA running the > CREATE TRANSFORM command, when using some plperl extensions for example. > > A mechanism allowing for the transform to only be used in some functions > but not others might be useful. The simplest such mechanism I can think > of is modeled against the PL/Java classpath facility as specified in the > SQL standard: you attach a classpath per schema. If we start adding granularity, then why not go all the way? I mean, we could do it in the following way 1) create named transforms CREATE [DEFAULT] TRANSFORM FOR LANGUAGE (...details...); 2) use it when declaring a function CREATE function ( IN [[USING] [TRANSFORM] ], INOUT [[USING] [IN] [TRANSFORM] ] [[USING] [OUT] [TRANSFORM] ], OUT [[USING] [TRANSFORM] ], ... ) LANGUAGE $$ $$; This approach allows full flexibility in using "old" packages, especially if we define old transform behaviour as "DEFAULT TRANSFORM" Default transforms also allow easy way for rewriting current type i/o conversions between languages into transforms. There are immediately a few transforms that I would find useful A) pass field data to language as pairs of (typeoid, typebin) this is useful for speed, especially if you do not want to use many of the passed arguments on most invocations B) pass field data in as (typeoid, typebin), except do not de-toast values but pass in the toast ids, so the function is free to use only parts of toasted values as it needs C) pass field data in as string, probably the default behaviour for languages like pl/tcl and pl/sh D) and then of course just having a sensible transforms for extension types like the current patch provides. > Worst case, that I really don't think we need, would be addressing that > per-argument: > > CREATE FUNCTION foo (hash hstore WITH TRANSFORM, kv hstore) … > > I certainly hope we don't need that, and sure can't imagine use cases > for that level of complexity at the time of writing this review. > A typical use case would be to have a "short" hstore always passed in as dictionary and have another possibly large hstore passed in as toast pointer. And if we want to have all type conversions between postgres and pls re-written as transforms, then we do need named transforms, not just one per (pl, type) pair. Also, if we allow flexibility, the it is probably a good idea to implement full flexibility first and then look at making usage easy after that, instead of adding flexibility in small steps. Once we have per-argument transforms in place, we can look at setting per-schema defaults for ease of use. As large part of this is actually abstracting i/o conversions out of pl function code, I think we should look at allowing the conversion functions to be written in the target pl language in addition to C. I'll see if I can resurrect my patch for support of "cstring" and "internal" types in pl/python function defs for this. -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Tue, 2013-11-26 at 18:10 +0100, Dimitri Fontaine wrote: > The problem is installing a set of extensions where some of them are > already using the new transform feature and some of them are not. We > need a way to cater with that, I think. Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. The only thing this doesn't give you is per-argument granularity, but I think the use cases for that are slim, and we don't have a good existing mechanism to attach arbitrary attributes to function arguments. Actually, I'd take this two steps further. First, make this parameter per-language, so something like plpython.use_transforms. Then it's up to the language implementation how they want to deal with this. A future new language could just ignore the whole issue and require transforms from the start. Second, depending on the choice of the language, this parameter could take three values: ignore | if available | require. That would allow users to set various kinds of strictness, for example if they want to be alerted that a language cannot deal with a particular type.
Re: [PATCH] Add transforms feature
От:
Dimitri Fontaine <dimitri@2ndQuadrant.fr>
Дата:
Peter Eisentraut writes: > Here is an idea. Add a GUC that basically says something like > use_transforms = on|off. You can then attach that to individual > functions, which is the right granularity, because only the function > knows whether its code expects transforms or not. But you can use the > full power of GUC to configure it any way you want. +1 > The only thing this doesn't give you is per-argument granularity, but I > think the use cases for that are slim, and we don't have a good existing > mechanism to attach arbitrary attributes to function arguments. +1 > Actually, I'd take this two steps further. > > First, make this parameter per-language, so something like > plpython.use_transforms. Then it's up to the language implementation > how they want to deal with this. A future new language could just > ignore the whole issue and require transforms from the start. I'm not sure about this level of granularity, but why not. > Second, depending on the choice of the language, this parameter could > take three values: ignore | if available | require. That would allow > users to set various kinds of strictness, for example if they want to be > alerted that a language cannot deal with a particular type. My understanding is that it always can deal with any particular type if you consider text based input/output, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: [PATCH] Add transforms feature
От:
Hannu Krosing <hannu@2ndQuadrant.com>
Дата:
On 12/06/2013 07:25 AM, Peter Eisentraut wrote: > On Tue, 2013-11-26 at 18:10 +0100, Dimitri Fontaine wrote: >> The problem is installing a set of extensions where some of them are >> already using the new transform feature and some of them are not. We >> need a way to cater with that, I think. > Here is an idea. Add a GUC that basically says something like > use_transforms = on|off. You can then attach that to individual > functions, which is the right granularity, because only the function > knows whether its code expects transforms or not. But you can use the > full power of GUC to configure it any way you want. It would requite the "old" extensions to be modified to have (SET use_transforms = off) in all their definitions so that they would not accidentally be called with use_transforms = on from "new" functions, but else it seems like a good way to get it done without too much effort. > The only thing this doesn't give you is per-argument granularity, but I > think the use cases for that are slim, and we don't have a good existing > mechanism to attach arbitrary attributes to function arguments. Agreed. And we are quite unlikely to need multiple transforms for the same type in the same language. > Actually, I'd take this two steps further. > > First, make this parameter per-language, so something like > plpython.use_transforms. Then it's up to the language implementation > how they want to deal with this. A future new language could just > ignore the whole issue and require transforms from the start. I do not really see much need for this, as it will need to be set for each individual function anyway. Actually what we could do is just declare a new "language" for this so functions declared with "LANGUAGE plpythonu" will not be using transforms and those with "LANGUAGE plpythonuxf" will use it. This would only need one extra function to be defined in source code, namely the compile function for the "new" language". Some not-transforms-related wild ideas follow :) Adding a new language would also be a good way to fix the bad syntax choices in pl/python which require code manipulation before compiling . I came up with this idea after seeing how pl/jsv8 supports multiple JavaScript-based languages (standard JavaScript, CoffeeScript, LiveScript) from the same codebase. Taking the plv8 ideas further we could also create a JavaScript-based "sandboxed python" using thins like skulpt and pyjamas which compile python source code to JavaScript VM and inherit all the sandboxing of v8. Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [PATCH] Add transforms feature
On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: > > Here is an idea. Add a GUC that basically says something like > > use_transforms = on|off. You can then attach that to individual > > functions, which is the right granularity, because only the function > > knows whether its code expects transforms or not. But you can use > the > > full power of GUC to configure it any way you want. Here is an updated patch that implements this, makes some of the documentation improvements that you suggested, and rebases everything.
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut wrote: > On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: >> > Here is an idea. Add a GUC that basically says something like >> > use_transforms = on|off. You can then attach that to individual >> > functions, which is the right granularity, because only the function >> > knows whether its code expects transforms or not. But you can use >> the >> > full power of GUC to configure it any way you want. > > Here is an updated patch that implements this, makes some of the > documentation improvements that you suggested, and rebases everything. I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
От:
Hannu Krosing <hannu@2ndQuadrant.com>
Дата:
On 12/11/2013 01:40 PM, Robert Haas wrote: > On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut wrote: >> On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: >>>> Here is an idea. Add a GUC that basically says something like >>>> use_transforms = on|off. You can then attach that to individual >>>> functions, which is the right granularity, because only the function >>>> knows whether its code expects transforms or not. But you can use >>> the >>>> full power of GUC to configure it any way you want. >> Here is an updated patch that implements this, makes some of the >> documentation improvements that you suggested, and rebases everything. > I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. > It should work ok if we could somehow check that the GUC is set on the function and fall back to session GUC in case it is not. Not sure if this is possible though. The need from this arises from calling other functions from a new func. At the moment if there is a new function defined as CREATE FUNCTION f_uses_xforms() AS $$ ... $$ SET use_transforms=on; calls a legacy function which will break if transforms are used then the _old_ function declaration needs to be modified to add (use_transforms=off) It is much easier than debugging/rewriting the function, but this is something I'd like us to be able to avoid. PS. maybe we could resurrect the WITH (attribute, ...) available in CREATE FUNCTION syntax for passing function-specific flags ? Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Wed, Dec 11, 2013 at 9:19 AM, Hannu Krosing wrote: > On 12/11/2013 01:40 PM, Robert Haas wrote: >> On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut wrote: >>> On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: >>>>> Here is an idea. Add a GUC that basically says something like >>>>> use_transforms = on|off. You can then attach that to individual >>>>> functions, which is the right granularity, because only the function >>>>> knows whether its code expects transforms or not. But you can use >>>> the >>>>> full power of GUC to configure it any way you want. >>> Here is an updated patch that implements this, makes some of the >>> documentation improvements that you suggested, and rebases everything. >> I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. >> > It should work ok if we could somehow check that the GUC is set > on the function and fall back to session GUC in case it is not. > > Not sure if this is possible though. > > The need from this arises from calling other functions from a new func. > At the moment if there is a new function defined as > > CREATE FUNCTION f_uses_xforms() AS $$ ... $$ SET use_transforms=on; > > calls a legacy function which will break if transforms are used then the > _old_ function declaration needs to be modified to add (use_transforms=off) Yeah, exactly. > It is much easier than debugging/rewriting the function, but this is > something I'd like us to be able to avoid. > > PS. maybe we could resurrect the WITH (attribute, ...) available in > CREATE FUNCTION syntax for passing function-specific flags ? It's a thought. Or you could put some annotation in the function body, as we do in PL/pgsql with the #option syntax. Of course, making everyone decorate their new functions with references to the transforms they want to use isn't wonderful either, but it might be good at least to have the option. You could allow the use of all installed transforms by default, but let people say WITH (transforms='') if they don't want to use them or WITH (transforms='comma, separated, list') if the want to require certain ones. Unfortunately, that'll probably mean that virtually all portable code for procedural languages has to include some form of this incantation, just as any nearly any PL/pgsql function has to include SET search_path = '' if it wants to be not trivially subvertable. It's annoying to grow more such decoration, but the alternative seems to be hoping that nobody wants to write portable code that uses non-core types, and that doesn't seem better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Robert Haas writes: > On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut wrote: >> Here is an updated patch that implements this, makes some of the >> documentation improvements that you suggested, and rebases everything. > I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. We should have learned by now that those are usually a bad idea. In this case, we've got changes in the behavior of function calling, which seems like not only a nightmare for debugging but a fertile source of security issues. regards, tom lane
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Wed, 2013-12-11 at 11:07 -0500, Tom Lane wrote: > We should have learned by now that those are usually a bad idea. > In this case, we've got changes in the behavior of function calling, > which seems like not only a nightmare for debugging but a fertile > source of security issues. I note that this is the same mechanism that we have elaborately designed for *avoiding* security issues from search_path.
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On Wed, 2013-12-11 at 09:47 -0500, Robert Haas wrote: > Of course, making everyone decorate their new functions with > references to the transforms they want to use isn't wonderful either, > but it might be good at least to have the option. You could allow the > use of all installed transforms by default, but let people say WITH > (transforms='') if they don't want to use them or WITH > (transforms='comma, separated, list') if the want to require certain > ones. I'll try to implement something like that.
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Fri, Jan 10, 2014 at 10:40 PM, Peter Eisentraut wrote: > On Wed, 2013-12-11 at 11:07 -0500, Tom Lane wrote: >> We should have learned by now that those are usually a bad idea. >> In this case, we've got changes in the behavior of function calling, >> which seems like not only a nightmare for debugging but a fertile >> source of security issues. > > I note that this is the same mechanism that we have elaborately designed > for *avoiding* security issues from search_path. And it works like crap. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
On Fri, 2014-01-10 at 22:46 -0500, Peter Eisentraut wrote:
> On Wed, 2013-12-11 at 09:47 -0500, Robert Haas wrote:
> > Of course, making everyone decorate their new functions with
> > references to the transforms they want to use isn't wonderful either,
> > but it might be good at least to have the option. You could allow the
> > use of all installed transforms by default, but let people say WITH
> > (transforms='') if they don't want to use them or WITH
> > (transforms='comma, separated, list') if the want to require certain
> > ones.
>
> I'll try to implement something like that.
So it turns out the SQL standard actually specifies it more or less that
way.
What I have implemented now is that you can attach a clause TRANSFORM
{ ALL | NONE | FOR TYPE xxx, ... } to CREATE FUNCTION, which is recorded
in pg_proc, so there is no interference by run-time parameters. I kept
the use_transforms parameter to set the default value (again, at CREATE
FUNCTION time). pg_dump never dumps TRANSFORM ALL, always the resolved
type list, so you get the exact behavior back after restores or
upgrades.
The only place where this currently breaks is SPI calls inside
functions, because CREATE FUNCTION doesn't know about them. That can be
worked around by providing an explicit type list, but that would in turn
interfere with what you want to do with the parameter list. Also, there
is no way to attach a TRANSFORM clause to inline calls (DO).
The attached patch will probably fail to apply because of the pg_proc
changes. So if you want to try it out, look into the header for the Git
hash it was based off. I'll produce a properly merged version when this
approach is validated. Also, some implementation details could probably
take some revising after a couple of nights of sleep. ;-)
Re: [PATCH] Add transforms feature
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2014-01-15 21:13:18 -0500, Peter Eisentraut wrote:
> The attached patch will probably fail to apply because of the pg_proc
> changes. So if you want to try it out, look into the header for the Git
> hash it was based off. I'll produce a properly merged version when this
> approach is validated. Also, some implementation details could probably
> take some revising after a couple of nights of sleep. ;-)
You've slept since? ;)
So, I am only doign a look through the patch, to see where it has gone
in the past year.
> index 4e476c3..5ac9f05 100644
> --- a/src/Makefile.shlib
> +++ b/src/Makefile.shlib
> @@ -133,7 +133,7 @@ ifeq ($(PORTNAME), darwin)
> else
> # loadable module
> DLSUFFIX = .so
> - LINK.shared = $(COMPILER) -bundle -multiply_defined suppress
> + LINK.shared = $(COMPILER) -bundle -multiply_defined suppress -Wl,-undefined,dynamic_lookup
> endif
> BUILD.exports = $(AWK) '/^[^\#]/ {printf "_%s\n",$$1}' $< >$@
> exports_file = $(SHLIB_EXPORTS:%.txt=%.list)
Hm. Do the linkers on other platforms support this behaviour? Linux
does, by default, but I have zap clue about the rest.
Why do we need this? I guess because a transform from e.g. hstore to $pl
needs symbols out of hstore.so and the $pl?
I wonder if it woudln't be better to rely on explicit symbol lookups for
this. That'd avoid the need for the order dependency and linker changes
like this.
> + case OBJECT_TRANSFORM:
> + {
> + TypeName *typename = (TypeName *) linitial(objname);
> + char *langname = (char *) linitial(objargs);
> + Oid type_id = typenameTypeId(NULL, typename);
> + Oid lang_id = get_language_oid(langname, false);
> +
> + address.classId = TransformRelationId;
> + address.objectId =
> + get_transform_oid(type_id, lang_id, missing_ok);
> + address.objectSubId = 0;
> + }
> + break;
Hm. I wonder if missing_ok should be forwarded to get_language_oid() and
(by changing the way things are done atm) to typenameTypeId?
> + case OCLASS_TRANSFORM:
> + {
> + HeapTuple trfTup;
> + Form_pg_transform trfForm;
> +
> + trfTup = SearchSysCache1(TRFOID,
> + ObjectIdGetDatum(object->objectId));
> + if (!HeapTupleIsValid(trfTup))
> + elog(ERROR, "could not find tuple for transform %u",
> + object->objectId);
> +
> + trfForm = (Form_pg_transform) GETSTRUCT(trfTup);
> +
> + appendStringInfo(&buffer, _("transform for %s language %s"),
> + format_type_be(trfForm->trftype),
> + get_language_name(trfForm->trflang, false));
> +
> + ReleaseSysCache(trfTup);
> + break;
> + }
> +
Why deviate from the usual 'cache lookup failed for ..'? elog doesn't
translate so it's not particular relevant, but ...
> referenced.objectSubId = 0;
> recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
>
> + /* dependency on transform used by return type, if any */
> + if ((trfid = get_transform_oid(returnType, languageObjectId, true)))
> + {
> + referenced.classId = TransformRelationId;
> + referenced.objectId = trfid;
> + referenced.objectSubId = 0;
> + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
> + }
> +
Should be compared to InvalidOid imo, rather than implicitly assuming
that InvalidOid evaluates to false.
> +/*
> + * CREATE TRANSFORM
> + */
> +Oid
> +CreateTransform(CreateTransformStmt *stmt)
> +{
...
> + if (!pg_type_ownercheck(typeid, GetUserId()))
> + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeid);
> +
> + aclresult = pg_type_aclcheck(typeid, GetUserId(), ACL_USAGE);
> + if (aclresult != ACLCHECK_OK)
> + aclcheck_error_type(aclresult, typeid);
> +
> + /*
> + * Get the language
> + */
> + langid = get_language_oid(stmt->lang, false);
> +
> + aclresult = pg_language_aclcheck(langid, GetUserId(), ACL_USAGE);
> + if (aclresult != ACLCHECK_OK)
> + aclcheck_error(aclresult, ACL_KIND_LANGUAGE, stmt->lang);
Hm. Is USAGE really sufficient here? Should we possibly make it
dependant on lanpltrusted like CreateFunction() does?
> + if (stmt->fromsql)
> + {
> + fromsqlfuncid = LookupFuncNameTypeNames(stmt->fromsql->funcname, stmt->fromsql->funcargs, false);
> +
> + if (!pg_proc_ownercheck(fromsqlfuncid, GetUserId()))
> + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(stmt->fromsql->funcname));
Why isn't EXECUTE sufficient here?
> + aclresult = pg_proc_aclcheck(fromsqlfuncid, GetUserId(), ACL_EXECUTE);
> + if (aclresult != ACLCHECK_OK)
> + aclcheck_error(aclresult, ACL_KIND_PROC, NameListToString(stmt->fromsql->funcname));
> +
> + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(fromsqlfuncid));
> + if (!HeapTupleIsValid(tuple))
> + elog(ERROR, "cache lookup failed for function %u", fromsqlfuncid);
> + procstruct = (Form_pg_proc) GETSTRUCT(tuple);
> + if (procstruct->prorettype != INTERNALOID)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> + errmsg("return data type of FROM SQL function must be \"internal\"")));
> + check_transform_function(procstruct);
> + ReleaseSysCache(tuple);
So, this can be used to call a function that takes INTERNAL, and returns
INTERNAL. Isn't that normally reserved for superusers? I think this at
the very least needs to be an ownercheck on the function?
> @@ -66,6 +66,7 @@ CATALOG(pg_proc,1255) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81) BKI_SCHEMA_MACRO
> text proargnames[1]; /* parameter names (NULL if no names) */
> pg_node_tree proargdefaults;/* list of expression trees for argument
> * defaults (NULL if none) */
> + Oid protrftypes[1] /* types for which to apply transforms */
> text prosrc; /* procedure source text */
> text probin; /* secondary procedure info (can be NULL) */
> text proconfig[1]; /* procedure-local GUC settings */
I wonder if this shouldn't rather be a array that lists the transform to
be used for every single column akin to proargtypes or such. That's
going to make life easier for pl developers.
> /**********************************************************************
> @@ -1260,6 +1264,7 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc,
> bool *isnull)
> {
> FmgrInfo tmp;
> + Oid funcid;
>
> /* we might recurse */
> check_stack_depth();
> @@ -1283,6 +1288,8 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc,
> /* must call typinput in case it wants to reject NULL */
> return InputFunctionCall(finfo, NULL, typioparam, typmod);
> }
> + else if ((funcid = get_transform_tosql(typid, current_call_data->prodesc->lang_oid)))
> + return OidFunctionCall1(funcid, PointerGetDatum(sv));
> else if (SvROK(sv))
> {
> /* handle references */
Am I missing something here? You're not looking at the proc's
transforms, but just lookup the general ones? Same for output and such.
Looks like you forgot to update this.
> for (i = 0; i < desc->nargs; i++)
> {
> if (fcinfo->argnull[i])
> @@ -2055,9 +2075,16 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc,
> else
> {
> SV *sv;
> + Oid funcid;
>
> if (OidIsValid(desc->arg_arraytype[i]))
> sv = plperl_ref_from_pg_array(fcinfo->arg[i], desc->arg_arraytype[i]);
> + else if (list_member_oid(current_call_data->prodesc->trftypes, argtypes[i]))
> + {
> + funcid = get_transform_fromsql(argtypes[i], current_call_data->prodesc->lang_oid);
> + Assert(funcid); // TODO
> + sv = (SV *) DatumGetPointer(OidFunctionCall1(funcid, fcinfo->arg[i]));
> + }
This is the behaviour I'd really would like to avoid. Searching an array
for every parameter sucks. I think this really should be a vector with
one element per argument. Yes, that's going to require special handling
of the return type, but whatever.
So, I lost my motiviation here. I like this version *much* more than the
state of a year ago. I think there's a fair amount of work required
here, but it seems to be dilligence that's required, not redesign. But I
still don't think that's doable within the next couple of days?
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [PATCH] Add transforms feature
От:
Andres Freund <andres@2ndquadrant.com>
Дата:
On 2014-04-05 00:21:47 +0200, Andres Freund wrote: > On 2014-01-15 21:13:18 -0500, Peter Eisentraut wrote: > > The attached patch will probably fail to apply because of the pg_proc > > changes. So if you want to try it out, look into the header for the Git > > hash it was based off. I'll produce a properly merged version when this > > approach is validated. Also, some implementation details could probably > > take some revising after a couple of nights of sleep. ;-) > > You've slept since? ;) > > So, I am only doign a look through the patch, to see where it has gone > in the past year. > ... So, unless somebody protests PDQ, I am going to mark this "Returned with Feedback". Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: [PATCH] Add transforms feature
I have an updated patch for this. At the end of the 2014-01 commit
fest, it seems that the design was generally considered OK.
This patch is rebased, has some updates and some bug fixes.
Responses to the last review below.
On 4/4/14 6:21 PM, Andres Freund wrote:
>> index 4e476c3..5ac9f05 100644
>> --- a/src/Makefile.shlib
>> +++ b/src/Makefile.shlib
>> @@ -133,7 +133,7 @@ ifeq ($(PORTNAME), darwin)
>> else
>> # loadable module
>> DLSUFFIX = .so
>> - LINK.shared = $(COMPILER) -bundle -multiply_defined suppress
>> + LINK.shared = $(COMPILER) -bundle -multiply_defined suppress -Wl,-undefined,dynamic_lookup
>> endif
>> BUILD.exports = $(AWK) '/^[^\#]/ {printf "_%s\n",$$1}' $< >$@
>> exports_file = $(SHLIB_EXPORTS:%.txt=%.list)
>
> Hm. Do the linkers on other platforms support this behaviour? Linux
> does, by default, but I have zap clue about the rest.
I think all other platforms do this by default, or can be made to do so.
> Why do we need this? I guess because a transform from e.g. hstore to $pl
> needs symbols out of hstore.so and the $pl?
>
> I wonder if it woudln't be better to rely on explicit symbol lookups for
> this. That'd avoid the need for the order dependency and linker changes
> like this.
That seems quite difficult. For example, hstore has things like
HS_COUNT() and HS_VAL(), which aren't even symbols.
>> + case OBJECT_TRANSFORM:
>> + {
>> + TypeName *typename = (TypeName *) linitial(objname);
>> + char *langname = (char *) linitial(objargs);
>> + Oid type_id = typenameTypeId(NULL, typename);
>> + Oid lang_id = get_language_oid(langname, false);
>> +
>> + address.classId = TransformRelationId;
>> + address.objectId =
>> + get_transform_oid(type_id, lang_id, missing_ok);
>> + address.objectSubId = 0;
>> + }
>> + break;
>
> Hm. I wonder if missing_ok should be forwarded to get_language_oid() and
> (by changing the way things are done atm) to typenameTypeId?
done
>> + case OCLASS_TRANSFORM:
>> + {
>> + HeapTuple trfTup;
>> + Form_pg_transform trfForm;
>> +
>> + trfTup = SearchSysCache1(TRFOID,
>> + ObjectIdGetDatum(object->objectId));
>> + if (!HeapTupleIsValid(trfTup))
>> + elog(ERROR, "could not find tuple for transform %u",
>> + object->objectId);
>> +
>> + trfForm = (Form_pg_transform) GETSTRUCT(trfTup);
>> +
>> + appendStringInfo(&buffer, _("transform for %s language %s"),
>> + format_type_be(trfForm->trftype),
>> + get_language_name(trfForm->trflang, false));
>> +
>> + ReleaseSysCache(trfTup);
>> + break;
>> + }
>> +
>
> Why deviate from the usual 'cache lookup failed for ..'? elog doesn't
> translate so it's not particular relevant, but ...
That's how the surrounding code does it.
>> referenced.objectSubId = 0;
>> recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
>>
>> + /* dependency on transform used by return type, if any */
>> + if ((trfid = get_transform_oid(returnType, languageObjectId, true)))
>> + {
>> + referenced.classId = TransformRelationId;
>> + referenced.objectId = trfid;
>> + referenced.objectSubId = 0;
>> + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
>> + }
>> +
>
> Should be compared to InvalidOid imo, rather than implicitly assuming
> that InvalidOid evaluates to false.
I think it's widely assumed that InvalidOid is false.
>> +/*
>> + * CREATE TRANSFORM
>> + */
>> +Oid
>> +CreateTransform(CreateTransformStmt *stmt)
>> +{
> ...
>> + if (!pg_type_ownercheck(typeid, GetUserId()))
>> + aclcheck_error_type(ACLCHECK_NOT_OWNER, typeid);
>> +
>> + aclresult = pg_type_aclcheck(typeid, GetUserId(), ACL_USAGE);
>> + if (aclresult != ACLCHECK_OK)
>> + aclcheck_error_type(aclresult, typeid);
>> +
>> + /*
>> + * Get the language
>> + */
>> + langid = get_language_oid(stmt->lang, false);
>> +
>> + aclresult = pg_language_aclcheck(langid, GetUserId(), ACL_USAGE);
>> + if (aclresult != ACLCHECK_OK)
>> + aclcheck_error(aclresult, ACL_KIND_LANGUAGE, stmt->lang);
>
> Hm. Is USAGE really sufficient here? Should we possibly make it
> dependant on lanpltrusted like CreateFunction() does?
It could be done, but I don't see why it's necessary. If the language
isn't trusted, why grant USAGE on it?
>> + if (stmt->fromsql)
>> + {
>> + fromsqlfuncid = LookupFuncNameTypeNames(stmt->fromsql->funcname, stmt->fromsql->funcargs, false);
>> +
>> + if (!pg_proc_ownercheck(fromsqlfuncid, GetUserId()))
>> + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(stmt->fromsql->funcname));
>
> Why isn't EXECUTE sufficient here?
because of the below
>> + aclresult = pg_proc_aclcheck(fromsqlfuncid, GetUserId(), ACL_EXECUTE);
>> + if (aclresult != ACLCHECK_OK)
>> + aclcheck_error(aclresult, ACL_KIND_PROC, NameListToString(stmt->fromsql->funcname));
>> +
>> + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(fromsqlfuncid));
>> + if (!HeapTupleIsValid(tuple))
>> + elog(ERROR, "cache lookup failed for function %u", fromsqlfuncid);
>> + procstruct = (Form_pg_proc) GETSTRUCT(tuple);
>> + if (procstruct->prorettype != INTERNALOID)
>> + ereport(ERROR,
>> + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>> + errmsg("return data type of FROM SQL function must be \"internal\"")));
>> + check_transform_function(procstruct);
>> + ReleaseSysCache(tuple);
>
> So, this can be used to call a function that takes INTERNAL, and returns
> INTERNAL. Isn't that normally reserved for superusers? I think this at
> the very least needs to be an ownercheck on the function?
exactly, see above
>> @@ -66,6 +66,7 @@ CATALOG(pg_proc,1255) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81) BKI_SCHEMA_MACRO
>> text proargnames[1]; /* parameter names (NULL if no names) */
>> pg_node_tree proargdefaults;/* list of expression trees for argument
>> * defaults (NULL if none) */
>> + Oid protrftypes[1] /* types for which to apply transforms */
>> text prosrc; /* procedure source text */
>> text probin; /* secondary procedure info (can be NULL) */
>> text proconfig[1]; /* procedure-local GUC settings */
>
> I wonder if this shouldn't rather be a array that lists the transform to
> be used for every single column akin to proargtypes or such. That's
> going to make life easier for pl developers.
That would allow using different transforms for different arguments of
the same type, which I don't want to allow, and PLs might not be able to
handle.
I understand where you are coming from, but the alternative seems worse.
>> /**********************************************************************
>> @@ -1260,6 +1264,7 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc,
>> bool *isnull)
>> {
>> FmgrInfo tmp;
>> + Oid funcid;
>>
>> /* we might recurse */
>> check_stack_depth();
>> @@ -1283,6 +1288,8 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc,
>> /* must call typinput in case it wants to reject NULL */
>> return InputFunctionCall(finfo, NULL, typioparam, typmod);
>> }
>> + else if ((funcid = get_transform_tosql(typid, current_call_data->prodesc->lang_oid)))
>> + return OidFunctionCall1(funcid, PointerGetDatum(sv));
>> else if (SvROK(sv))
>> {
>> /* handle references */
>
> Am I missing something here? You're not looking at the proc's
> transforms, but just lookup the general ones? Same for output and such.
>
> Looks like you forgot to update this.
fixed
Re: [PATCH] Add transforms feature
От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Peter Eisentraut writes: > On 4/4/14 6:21 PM, Andres Freund wrote: > + /* dependency on transform used by return type, if any */ > + if ((trfid = get_transform_oid(returnType, languageObjectId, true))) >> Should be compared to InvalidOid imo, rather than implicitly assuming >> that InvalidOid evaluates to false. > I think it's widely assumed that InvalidOid is false. That's not the point; the point is that some nonzero number of compilers (and probably Coverity too) will warn about this construct, on the not unreasonable grounds that = might be a typo for ==. (Those extra parens might satisfy gcc, but not other tools.) Please put in an explicit comparison of the assignment result, as is done in approximately 100% of the other places where this idiom appears in Postgres. regards, tom lane
Re: [PATCH] Add transforms feature
От:
Michael Paquier <michael.paquier@gmail.com>
Дата:
On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut wrote: > fixed This patch needs a rebase, it does not apply correctly in a couple of places on latest HEAD (699300a): ./src/include/catalog/catversion.h.rej ./src/include/catalog/pg_proc.h.rej ./src/pl/plpython/plpy_procedure.c.rej Regards, -- Michael
Re: [PATCH] Add transforms feature
От:
Michael Paquier <michael.paquier@gmail.com>
Дата:
On Mon, Dec 22, 2014 at 12:19 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> fixed
This patch needs a rebase, it does not apply correctly in a couple of
places on latest HEAD (699300a):
./src/include/catalog/catversion.h.rej
./src/include/catalog/pg_proc.h.rej
./src/pl/plpython/plpy_procedure.c.rej
I moved this patch to 2015-02 to not lose track of it and because it did not receive much reviews...
--
--
Michael
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
Hi
I am checking this patch, but it is broken stillRegards
Pavel
2015-02-13 8:14 GMT+01:00 Michael Paquier <michael.paquier@gmail.com>:
On Mon, Dec 22, 2014 at 12:19 PM, Michael Paquier <michael.paquier@gmail.com> wrote:On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> fixed
This patch needs a rebase, it does not apply correctly in a couple of
places on latest HEAD (699300a):
./src/include/catalog/catversion.h.rej
./src/include/catalog/pg_proc.h.rej
./src/pl/plpython/plpy_procedure.c.rejI moved this patch to 2015-02 to not lose track of it and because it did not receive much reviews...
--Michael
Re: [PATCH] Add transforms feature
On 3/6/15 9:56 AM, Pavel Stehule wrote: > Hi > > I am checking this patch, but it is broken still Here is an updated patch. (Note that because of the large pg_proc.h changes, it is likely to get outdated again soon. But for reviewing, you can always apply it against an older version of master.)
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
Hi
I am looking to code.Oid protrftypes[1]; /* types for which to apply transforms */
DO '' LANGUAGE plperl;
SELECT NULL::hstore;
use load plperl; load hstore; instead
3. missing documentation for new contrib modules,
4. pg_dump - wrong comment
+<-----><------>/*
+<-----><------> * protrftypes was added at v9.4
+<-----><------> */
+<-----><------>/*
+<-----><------> * protrftypes was added at v9.4
+<-----><------> */
4. Why guc-use-transforms? Is there some possible negative side effect of transformations, so we have to disable it? If somebody don't would to use some transformations, then he should not to install some specific transformation.
5. I don't understand to motivation for introduction of protrftypes in pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from documentation, and examples in contribs works without it. Is it this functionality really necessary? Missing tests, missing examples.
Regards
Pavel
2015-03-08 16:34 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
On 3/6/15 9:56 AM, Pavel Stehule wrote:
> Hi
>
> I am checking this patch, but it is broken still
Here is an updated patch.
(Note that because of the large pg_proc.h changes, it is likely to get
outdated again soon. But for reviewing, you can always apply it against
an older version of master.)
Re: [PATCH] Add transforms feature
От:
Peter Eisentraut <peter_e@gmx.net>
Дата:
On 3/12/15 8:12 AM, Pavel Stehule wrote: > 1. fix missing semicolon pg_proc.h > > Oid protrftypes[1]; /* types for which to apply > transforms */ Darn, I thought I had fixed that. > 2. strange load lib by in sql scripts: > > DO '' LANGUAGE plperl; > SELECT NULL::hstore; > > use load plperl; load hstore; instead OK > 3. missing documentation for new contrib modules, OK > 4. pg_dump - wrong comment > > +<-----><------>/* > +<-----><------> * protrftypes was added at v9.4 > +<-----><------> */ OK > 4. Why guc-use-transforms? Is there some possible negative side effect > of transformations, so we have to disable it? If somebody don't would to > use some transformations, then he should not to install some specific > transformation. Well, there was extensive discussion last time around where people disagreed with that assertion. > 5. I don't understand to motivation for introduction of protrftypes in > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from > documentation, and examples in contribs works without it. Is it this > functionality really necessary? Missing tests, missing examples. Again, this came out from the last round of discussion that people wanted to select which transforms to use and that the function needs to remember that choice, so it doesn't depend on whether a transform happens to be installed or not. Also, it's in the SQL standard that way (by analogy).
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
2015-03-17 2:51 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
On 3/12/15 8:12 AM, Pavel Stehule wrote:
> 1. fix missing semicolon pg_proc.h
>
> Oid protrftypes[1]; /* types for which to apply
> transforms */
Darn, I thought I had fixed that.
> 2. strange load lib by in sql scripts:
>
> DO '' LANGUAGE plperl;
> SELECT NULL::hstore;
>
> use load plperl; load hstore; instead
OK
> 3. missing documentation for new contrib modules,
OK
> 4. pg_dump - wrong comment
>
> +<-----><------>/*
> +<-----><------> * protrftypes was added at v9.4
> +<-----><------> */
OK
> 4. Why guc-use-transforms? Is there some possible negative side effect
> of transformations, so we have to disable it? If somebody don't would to
> use some transformations, then he should not to install some specific
> transformation.
Well, there was extensive discussion last time around where people
disagreed with that assertion.
I don't like it, but I can accept it - it should not to impact a functionality.
> 5. I don't understand to motivation for introduction of protrftypes in
> pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from
> documentation, and examples in contribs works without it. Is it this
> functionality really necessary? Missing tests, missing examples.
Again, this came out from the last round of discussion that people
wanted to select which transforms to use and that the function needs to
remember that choice, so it doesn't depend on whether a transform
happens to be installed or not. Also, it's in the SQL standard that way
(by analogy).
I am sorry, I didn't discuss this topic and I don't agree so it is good idea. I looked to standard, and I found CREATE TRANSFORM part there. But nothing else.
Personally I am thinking, so it is terrible wrong idea, unclean, redundant. If we define TRANSFORM, then we should to use it. Not prepare bypass in same moment.
Can be it faster, safer with it? I don't think.
Regards
Pavel
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Mon, Mar 16, 2015 at 9:51 PM, Peter Eisentraut wrote: >> 4. Why guc-use-transforms? Is there some possible negative side effect >> of transformations, so we have to disable it? If somebody don't would to >> use some transformations, then he should not to install some specific >> transformation. > > Well, there was extensive discussion last time around where people > disagreed with that assertion. I think we need to the ability to control it per-function, but having a global disabling knob on top of that doesn't seem especially useful. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
Here is an updated patch. On 3/17/15 1:11 AM, Pavel Stehule wrote: > 2015-03-17 2:51 GMT+01:00 Peter Eisentraut >: > > On 3/12/15 8:12 AM, Pavel Stehule wrote: > > 1. fix missing semicolon pg_proc.h > > > > Oid protrftypes[1]; /* types for which to apply > > transforms */ > > Darn, I thought I had fixed that. Fixed. > > 2. strange load lib by in sql scripts: > > > > DO '' LANGUAGE plperl; > > SELECT NULL::hstore; > > > > use load plperl; load hstore; instead > > OK The reason I had actually not used LOAD is that LOAD requires a file name, and the file name of those extensions is an implementation detail. So it is less of a violation to just execute something from those modules rather than reach in and deal with the file directly. It's not terribly pretty either way, I admit. A proper fix would be to switch to lazy symbol resolution, but that would be a much bigger change. > > 3. missing documentation for new contrib modules, > > OK They actually are documented as part of the hstore and ltree modules already. > > 4. pg_dump - wrong comment > > > > +<-----><------>/* > > +<-----><------> * protrftypes was added at v9.4 > > +<-----><------> */ > > OK Fixed. > > 4. Why guc-use-transforms? Is there some possible negative side effect > > of transformations, so we have to disable it? If somebody don't would to > > use some transformations, then he should not to install some specific > > transformation. > > Well, there was extensive discussion last time around where people > disagreed with that assertion. > > > I don't like it, but I can accept it - it should not to impact a > functionality. Removed. > > 5. I don't understand to motivation for introduction of protrftypes in > > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from > > documentation, and examples in contribs works without it. Is it this > > functionality really necessary? Missing tests, missing examples. > > Again, this came out from the last round of discussion that people > wanted to select which transforms to use and that the function needs to > remember that choice, so it doesn't depend on whether a transform > happens to be installed or not. Also, it's in the SQL standard that way > (by analogy). > > > I am sorry, I didn't discuss this topic and I don't agree so it is good > idea. I looked to standard, and I found CREATE TRANSFORM part there. But > nothing else. > > Personally I am thinking, so it is terrible wrong idea, unclean, > redundant. If we define TRANSFORM, then we should to use it. Not prepare > bypass in same moment. > > Can be it faster, safer with it? I don't think. Well, I don't think there is any point in reopening this discussion. This is a safety net of sorts that people wanted. You can argue that it would be more fun without it, but nobody else would agree. There is really no harm in keeping it. All the function lookup is mostly cached anyway. The only time this is really important is for pg_dump to be able to accurately restore function behavior.
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
2015-03-22 3:55 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
Here is an updated patch.
On 3/17/15 1:11 AM, Pavel Stehule wrote:
> 2015-03-17 2:51 GMT+01:00 Peter Eisentraut <peter_e@gmx.net
> <mailto:peter_e@gmx.net>>:
>
> On 3/12/15 8:12 AM, Pavel Stehule wrote:
> > 1. fix missing semicolon pg_proc.h
> >
> > Oid protrftypes[1]; /* types for which to apply
> > transforms */
>
> Darn, I thought I had fixed that.
Fixed.
> > 2. strange load lib by in sql scripts:
> >
> > DO '' LANGUAGE plperl;
> > SELECT NULL::hstore;
> >
> > use load plperl; load hstore; instead
>
> OK
The reason I had actually not used LOAD is that LOAD requires a file
name, and the file name of those extensions is an implementation detail.
So it is less of a violation to just execute something from those
modules rather than reach in and deal with the file directly.
It's not terribly pretty either way, I admit. A proper fix would be to
switch to lazy symbol resolution, but that would be a much bigger change.
> > 3. missing documentation for new contrib modules,
>
> OK
They actually are documented as part of the hstore and ltree modules
already.
> > 4. pg_dump - wrong comment
> >
> > +<-----><------>/*
> > +<-----><------> * protrftypes was added at v9.4
> > +<-----><------> */
>
> OK
Fixed.
> > 4. Why guc-use-transforms? Is there some possible negative side effect
> > of transformations, so we have to disable it? If somebody don't would to
> > use some transformations, then he should not to install some specific
> > transformation.
>
> Well, there was extensive discussion last time around where people
> disagreed with that assertion.
>
>
> I don't like it, but I can accept it - it should not to impact a
> functionality.
Removed.
> > 5. I don't understand to motivation for introduction of protrftypes in
> > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from
> > documentation, and examples in contribs works without it. Is it this
> > functionality really necessary? Missing tests, missing examples.
>
> Again, this came out from the last round of discussion that people
> wanted to select which transforms to use and that the function needs to
> remember that choice, so it doesn't depend on whether a transform
> happens to be installed or not. Also, it's in the SQL standard that way
> (by analogy).
>
>
> I am sorry, I didn't discuss this topic and I don't agree so it is good
> idea. I looked to standard, and I found CREATE TRANSFORM part there. But
> nothing else.
>
> Personally I am thinking, so it is terrible wrong idea, unclean,
> redundant. If we define TRANSFORM, then we should to use it. Not prepare
> bypass in same moment.
>
> Can be it faster, safer with it? I don't think.
Well, I don't think there is any point in reopening this discussion.
This is a safety net of sorts that people wanted. You can argue that it
would be more fun without it, but nobody else would agree. There is
really no harm in keeping it. All the function lookup is mostly cached
anyway. The only time this is really important is for pg_dump to be
able to accurately restore function behavior.
1. It add attribute to pg_proc, so impact is not minimal
2. Minimally it is not tested - there are no any test for this functionality
3. I'll reread a discuss about this design - Now I am thinking so this duality (in design) is wrong - worse in relatively critical part of Postgres.
I can mark this patch as "ready for commiter" with objection - It is task for commiter, who have to decide.
Regards
Pavel
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
2015-03-22 5:45 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
2015-03-22 3:55 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:Here is an updated patch.
On 3/17/15 1:11 AM, Pavel Stehule wrote:
> 2015-03-17 2:51 GMT+01:00 Peter Eisentraut <peter_e@gmx.net
> <mailto:peter_e@gmx.net>>:
>
> On 3/12/15 8:12 AM, Pavel Stehule wrote:
> > 1. fix missing semicolon pg_proc.h
> >
> > Oid protrftypes[1]; /* types for which to apply
> > transforms */
>
> Darn, I thought I had fixed that.
Fixed.
> > 2. strange load lib by in sql scripts:
> >
> > DO '' LANGUAGE plperl;
> > SELECT NULL::hstore;
> >
> > use load plperl; load hstore; instead
>
> OK
The reason I had actually not used LOAD is that LOAD requires a file
name, and the file name of those extensions is an implementation detail.
So it is less of a violation to just execute something from those
modules rather than reach in and deal with the file directly.
It's not terribly pretty either way, I admit. A proper fix would be to
switch to lazy symbol resolution, but that would be a much bigger change.
> > 3. missing documentation for new contrib modules,
>
> OK
They actually are documented as part of the hstore and ltree modules
already.
> > 4. pg_dump - wrong comment
> >
> > +<-----><------>/*
> > +<-----><------> * protrftypes was added at v9.4
> > +<-----><------> */
>
> OK
Fixed.
> > 4. Why guc-use-transforms? Is there some possible negative side effect
> > of transformations, so we have to disable it? If somebody don't would to
> > use some transformations, then he should not to install some specific
> > transformation.
>
> Well, there was extensive discussion last time around where people
> disagreed with that assertion.
>
>
> I don't like it, but I can accept it - it should not to impact a
> functionality.
Removed.
> > 5. I don't understand to motivation for introduction of protrftypes in
> > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from
> > documentation, and examples in contribs works without it. Is it this
> > functionality really necessary? Missing tests, missing examples.
>
> Again, this came out from the last round of discussion that people
> wanted to select which transforms to use and that the function needs to
> remember that choice, so it doesn't depend on whether a transform
> happens to be installed or not. Also, it's in the SQL standard that way
> (by analogy).
>
>
> I am sorry, I didn't discuss this topic and I don't agree so it is good
> idea. I looked to standard, and I found CREATE TRANSFORM part there. But
> nothing else.
>
> Personally I am thinking, so it is terrible wrong idea, unclean,
> redundant. If we define TRANSFORM, then we should to use it. Not prepare
> bypass in same moment.
>
> Can be it faster, safer with it? I don't think.
Well, I don't think there is any point in reopening this discussion.
This is a safety net of sorts that people wanted. You can argue that it
would be more fun without it, but nobody else would agree. There is
really no harm in keeping it. All the function lookup is mostly cached
anyway. The only time this is really important is for pg_dump to be
able to accurately restore function behavior.1. It add attribute to pg_proc, so impact is not minimal2. Minimally it is not tested - there are no any test for this functionality
I am sorry, there is tests
3. I'll reread a discuss about this design - Now I am thinking so this duality (in design) is wrong - worse in relatively critical part of Postgres.I can mark this patch as "ready for commiter" with objection - It is task for commiter, who have to decide.RegardsPavel
Re: [PATCH] Add transforms feature
От:
Pavel Stehule <pavel.stehule@gmail.com>
Дата:
2015-03-22 3:55 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
Here is an updated patch.
On 3/17/15 1:11 AM, Pavel Stehule wrote:
> 2015-03-17 2:51 GMT+01:00 Peter Eisentraut <peter_e@gmx.net
> <mailto:peter_e@gmx.net>>:
>
> On 3/12/15 8:12 AM, Pavel Stehule wrote:
> > 1. fix missing semicolon pg_proc.h
> >
> > Oid protrftypes[1]; /* types for which to apply
> > transforms */
>
> Darn, I thought I had fixed that.
Fixed.
> > 2. strange load lib by in sql scripts:
> >
> > DO '' LANGUAGE plperl;
> > SELECT NULL::hstore;
> >
> > use load plperl; load hstore; instead
>
> OK
The reason I had actually not used LOAD is that LOAD requires a file
name, and the file name of those extensions is an implementation detail.
So it is less of a violation to just execute something from those
modules rather than reach in and deal with the file directly.
It's not terribly pretty either way, I admit. A proper fix would be to
switch to lazy symbol resolution, but that would be a much bigger change.
ok, please, can comment the reason in test. little bit more verbose than "make sure the prerequisite libraries are loaded". There is not clean, why "LOAD" should not be used.
> > 3. missing documentation for new contrib modules,
>
> OK
They actually are documented as part of the hstore and ltree modules
already.
> > 4. pg_dump - wrong comment
> >
> > +<-----><------>/*
> > +<-----><------> * protrftypes was added at v9.4
> > +<-----><------> */
>
> OK
Fixed.
> > 4. Why guc-use-transforms? Is there some possible negative side effect
> > of transformations, so we have to disable it? If somebody don't would to
> > use some transformations, then he should not to install some specific
> > transformation.
>
> Well, there was extensive discussion last time around where people
> disagreed with that assertion.
>
>
> I don't like it, but I can accept it - it should not to impact a
> functionality.
Removed.
> > 5. I don't understand to motivation for introduction of protrftypes in
> > pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from
> > documentation, and examples in contribs works without it. Is it this
> > functionality really necessary? Missing tests, missing examples.
>
> Again, this came out from the last round of discussion that people
> wanted to select which transforms to use and that the function needs to
> remember that choice, so it doesn't depend on whether a transform
> happens to be installed or not. Also, it's in the SQL standard that way
> (by analogy).
>
>
> I am sorry, I didn't discuss this topic and I don't agree so it is good
> idea. I looked to standard, and I found CREATE TRANSFORM part there. But
> nothing else.
>
> Personally I am thinking, so it is terrible wrong idea, unclean,
> redundant. If we define TRANSFORM, then we should to use it. Not prepare
> bypass in same moment.
>
> Can be it faster, safer with it? I don't think.
Well, I don't think there is any point in reopening this discussion.
This is a safety net of sorts that people wanted. You can argue that it
would be more fun without it, but nobody else would agree. There is
really no harm in keeping it. All the function lookup is mostly cached
anyway. The only time this is really important is for pg_dump to be
able to accurately restore function behavior.
So I reread discussion about this topic and I can see some benefits of it. Still - what I dislike is the behave of TRANSFORM ALL. The fact, so newly installed transformations are not used for registered functions (and reregistration is needed) is unhappy. I understand, so it can depends on implementation requirements.
Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would to use transformations - then he have to explicitly enable it by "TRANSFORM FOR TYPE" ? It is safe and without possible user unexpectations.
Small issue - explicitly setted transformation types should be visible in \sf and \df+ commands.
Regards
Pavel
Re: [PATCH] Add transforms feature
On 3/22/15 5:46 AM, Pavel Stehule wrote: > Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would > to use transformations - then he have to explicitly enable it by > "TRANSFORM FOR TYPE" ? It is safe and without possible user unexpectations. Following our off-list conversation, here is a new patch that removes the TRANSFORM ALL/NONE clauses and requires an explicit list.
Re: [PATCH] Add transforms feature
2015-04-08 4:55 GMT+02:00 Peter Eisentraut <peter_e@gmx.net>:
On 3/22/15 5:46 AM, Pavel Stehule wrote:
> Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would
> to use transformations - then he have to explicitly enable it by
> "TRANSFORM FOR TYPE" ? It is safe and without possible user unexpectations.
Following our off-list conversation, here is a new patch that removes
the TRANSFORM ALL/NONE clauses and requires an explicit list.
Nice, thank you very much
I checked the description of this feature in other databases and in standard. There is little bit different situations - because there is not possibility to use external languages without transformations. In PostgreSQL we living without transformations long years, so we have to solve a co-existence old code (or functions without the using transformations) and new code (functions that use transformations). We have to solve a possible compatibility issues. The last design requires to specify explicitly list of types that are transformed. Nothing transformation is used by default (implicitly). So old code have to work without any issues and new code is clearly marked. Now I don't afraid of introduction of transformations. It is safe.
Review
----------
----------
1. What it does? - it introduce a secondary way, how to pass values between PostgreSQL and PL languages.
2. Does we would this feature? Surely - we would. It is safe way for passing complex types more cleverly and use it in PL more comfortably. Enhancing work with hstore in PLPerl, PLPython is long desired feature.
3. It can enforce some compatibility issues? No, last design is safe - the using transformation of any type must be explicitly enabled. It is clean what types will be transformed, and when transformations is required and when not.
4. I was able to apply patch cleanly - there are no compilation warnings.
5. This feature is documented well - new SQL statements, new system tables.
6. Code is clean and well documented.
7. This feature has enough regress tests - all tests passed without problems.
8. It requires pg_dump support. I checked it - no problems
I have not any objection. I'll mark it as ready for commit.
Minor issue is missing support for \sf command in psql. I wrote small patch that fix it.
Thank you very much for on this pretty nice feature.
Regards
Pavel
Re: [PATCH] Add transforms feature
On Tue, Apr 7, 2015 at 7:55 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
On 3/22/15 5:46 AM, Pavel Stehule wrote:
> Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would
> to use transformations - then he have to explicitly enable it by
> "TRANSFORM FOR TYPE" ? It is safe and without possible user unexpectations.
Following our off-list conversation, here is a new patch that removes
the TRANSFORM ALL/NONE clauses and requires an explicit list.
Hi Peter,
This commit is causing a compiler warning for me in non-cassert builds:
funcapi.c: In function 'get_func_trftypes':
funcapi.c:890: warning: unused variable 'procStruct'
Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it.
Cheers,
Jeff
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Tue, Apr 28, 2015 at 12:47 PM, Jeff Janes wrote: > This commit is causing a compiler warning for me in non-cassert builds: > > funcapi.c: In function 'get_func_trftypes': > funcapi.c:890: warning: unused variable 'procStruct' > > Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it. I took a stab at fixing this via a slightly different method. Let me know whether that worked. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [PATCH] Add transforms feature
От:
Jeff Janes <jeff.janes@gmail.com>
Дата:
On Wed, Apr 29, 2015 at 11:27 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Apr 28, 2015 at 12:47 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> This commit is causing a compiler warning for me in non-cassert builds:
>
> funcapi.c: In function 'get_func_trftypes':
> funcapi.c:890: warning: unused variable 'procStruct'
>
> Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it.
I took a stab at fixing this via a slightly different method. Let me
know whether that worked.
It worked, thanks.
Re: [PATCH] Add transforms feature
От:
Robert Haas <robertmhaas@gmail.com>
Дата:
On Wed, Apr 29, 2015 at 3:21 PM, Jeff Janes wrote: > On Wed, Apr 29, 2015 at 11:27 AM, Robert Haas wrote: >> On Tue, Apr 28, 2015 at 12:47 PM, Jeff Janes wrote: >> > This commit is causing a compiler warning for me in non-cassert builds: >> > >> > funcapi.c: In function 'get_func_trftypes': >> > funcapi.c:890: warning: unused variable 'procStruct' >> > >> > Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it. >> >> I took a stab at fixing this via a slightly different method. Let me >> know whether that worked. > > It worked, thanks. Cool. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company