Thread: 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.
Attachment
On Thu, Jun 14, 2012 at 3:42 PM, Peter Eisentraut <peter_e@gmx.net> 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
On Sat, Jun 16, 2012 at 7:15 PM, Jeff Janes <jeff.janes@gmail.com> wrote: > On Thu, Jun 14, 2012 at 3:42 PM, Peter Eisentraut <peter_e@gmx.net> 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
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
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.
Attachment
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.
Attachment
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
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
> 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
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=#
> 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
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
> 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
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.
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 transformfor 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
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
> 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
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
> 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
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.
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
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
> 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
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
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
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
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.
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
Peter Eisentraut <peter_e@gmx.net> 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
On 03/13/2013 09:54 AM, Dimitri Fontaine wrote: > Peter Eisentraut <peter_e@gmx.net> 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
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
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.
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 FUNCTIONplperl_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 FUNCTIONplperl_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 @@ <title>System Catalogs</title> </row> <row> + <entry><link linkend="catalog-pg-transform"><structname>pg_transform</structname></link></entry> + <entry>transforms (data type to procedural language conversions)</entry> + </row> + + <row> <entry><link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link></entry> <entry>triggers</entry> </row> @@ -5722,6 +5727,74 @@ <title><structname>pg_tablespace</> Columns</title> </sect1> + <sect1 id="catalog-pg-transform"> + <title><structname>pg_transform</structname></title> + + <indexterm zone="catalog-pg-transform"> + <primary>pg_transform</primary> + </indexterm> + + <para> + The catalog <structname>pg_transform</structname> stores information about + transforms, which are a mechanism to adapt data types to procedural + languages. See <xref linkend="sql-createtransform"> for more information. + </para> + + <table> + <title><structname>pg_transform</> Columns</title> + + <tgroup cols="4"> + <thead> + <row> + <entry>Name</entry> + <entry>Type</entry> + <entry>References</entry> + <entry>Description</entry> + </row> + </thead> + + <tbody> + <row> + <entry><structfield>trftype</structfield></entry> + <entry><type>oid</type></entry> + <entry><literal><link linkend="catalog-pg-type"><structname>pg_type</structname></link>.oid</literal></entry> + <entry>OID of the data type this transform is for</entry> + </row> + + <row> + <entry><structfield>trflang</structfield></entry> + <entry><type>oid</type></entry> + <entry><literal><link linkend="catalog-pg-language"><structname>pg_language</structname></link>.oid</literal></entry> + <entry>OID of the language this transform is for</entry> + </row> + + <row> + <entry><structfield>trffromsql</structfield></entry> + <entry><type>regproc</type></entry> + <entry><literal><link linkend="catalog-pg-proc"><structname>pg_proc</structname></link>.oid</literal></entry> + <entry> + The OID of the function to use when converting the data type for input + to the procedural language (e.g., function parameters). Zero is stored + if this operation is not supported. + </entry> + </row> + + <row> + <entry><structfield>trftosql</structfield></entry> + <entry><type>regproc</type></entry> + <entry><literal><link linkend="catalog-pg-proc"><structname>pg_proc</structname></link>.oid</literal></entry> + <entry> + The OID of the function to use when converting output from the + procedural language (e.g., return values) to the data type. Zero is + stored if this operation is not supported. + </entry> + </row> + </tbody> + </tgroup> + </table> + </sect1> + + <sect1 id="catalog-pg-trigger"> <title><structname>pg_trigger</structname></title> diff --git a/doc/src/sgml/hstore.sgml b/doc/src/sgml/hstore.sgml index 73c421d..a8f1537 100644 --- a/doc/src/sgml/hstore.sgml +++ b/doc/src/sgml/hstore.sgml @@ -597,6 +597,24 @@ <title>Compatibility</title> </sect2> <sect2> + <title>Transforms</title> + + <para> + Additional extensions are available that implement transforms for + the <type>hstore</type> type for the languages PL/Perl and PL/Python. The + extensions for PL/Perl are called <literal>hstore_plperl</literal> + and <literal>hstore_plperlu</literal>, for trusted and untrusted PL/Perl. + If you install these extensions, <type>hstore</type> values are + automatically mapped to Perl hashes. The extensions for PL/Python are + called <literal>hstore_plpythonu</literal>, <literal>hstore_plpython2u</literal>, + and <literal>hstore_plpython3u</literal> + (see <xref linkend="plpython-python23"> for the PL/Python naming + convention). If you install them, <type>hstore</type> values are + automatically mapped to Python dictionaries. + </para> + </sect2> + + <sect2> <title>Authors</title> <para> diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml index f5a0ac9..9f80d75 100644 --- a/doc/src/sgml/ltree.sgml +++ b/doc/src/sgml/ltree.sgml @@ -665,6 +665,21 @@ <title>Example</title> </sect2> <sect2> + <title>Transforms</title> + + <para> + Additional extensions are available that implement transforms for + the <type>ltree</type> type for PL/Python. The extensions are + called <literal>ltree_plpythonu</literal>, <literal>ltree_plpython2u</literal>, + and <literal>ltree_plpython3u</literal> + (see <xref linkend="plpython-python23"> for the PL/Python naming + convention). If you install them, <type>ltree</type> values are + automatically mapped to Python lists. (The reverse is currently not + supported, however.) + </para> + </sect2> + + <sect2> <title>Authors</title> <para> diff --git a/doc/src/sgml/ref/allfiles.sgml b/doc/src/sgml/ref/allfiles.sgml index 5846974..9ee8517 100644 --- a/doc/src/sgml/ref/allfiles.sgml +++ b/doc/src/sgml/ref/allfiles.sgml @@ -76,6 +76,7 @@ <!ENTITY createTable SYSTEM "create_table.sgml"> <!ENTITY createTableAs SYSTEM "create_table_as.sgml"> <!ENTITY createTableSpace SYSTEM "create_tablespace.sgml"> +<!ENTITY createTransform SYSTEM "create_transform.sgml"> <!ENTITY createTrigger SYSTEM "create_trigger.sgml"> <!ENTITY createTSConfig SYSTEM "create_tsconfig.sgml"> <!ENTITY createTSDictionary SYSTEM "create_tsdictionary.sgml"> @@ -116,6 +117,7 @@ <!ENTITY dropServer SYSTEM "drop_server.sgml"> <!ENTITY dropTable SYSTEM "drop_table.sgml"> <!ENTITY dropTableSpace SYSTEM "drop_tablespace.sgml"> +<!ENTITY dropTransform SYSTEM "drop_transform.sgml"> <!ENTITY dropTrigger SYSTEM "drop_trigger.sgml"> <!ENTITY dropTSConfig SYSTEM "drop_tsconfig.sgml"> <!ENTITY dropTSDictionary SYSTEM "drop_tsdictionary.sgml"> diff --git a/doc/src/sgml/ref/alter_extension.sgml b/doc/src/sgml/ref/alter_extension.sgml index 2dbba0c..f0fad1e 100644 --- a/doc/src/sgml/ref/alter_extension.sgml +++ b/doc/src/sgml/ref/alter_extension.sgml @@ -52,6 +52,7 @@ TEXT SEARCH DICTIONARY <replaceable class="PARAMETER">object_name</replaceable> | TEXT SEARCH PARSER <replaceable class="PARAMETER">object_name</replaceable> | TEXT SEARCH TEMPLATE <replaceable class="PARAMETER">object_name</replaceable> | + TRANSFORM FOR <replaceable>type_name</replaceable> LANGUAGE <replaceable>lang_name</replaceable> | TYPE <replaceable class="PARAMETER">object_name</replaceable> | VIEW <replaceable class="PARAMETER">object_name</replaceable> </synopsis> @@ -264,6 +265,26 @@ <title>Parameters</title> </para> </listitem> </varlistentry> + + <varlistentry> + <term><replaceable>type_name</replaceable></term> + + <listitem> + <para> + The name of the data type of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>lang_name</replaceable></term> + + <listitem> + <para> + The name of the language of the transform. + </para> + </listitem> + </varlistentry> </variablelist> </para> </refsect1> diff --git a/doc/src/sgml/ref/comment.sgml b/doc/src/sgml/ref/comment.sgml index e94dd4b..7aad302 100644 --- a/doc/src/sgml/ref/comment.sgml +++ b/doc/src/sgml/ref/comment.sgml @@ -54,6 +54,7 @@ TEXT SEARCH DICTIONARY <replaceable class="PARAMETER">object_name</replaceable> | TEXT SEARCH PARSER <replaceable class="PARAMETER">object_name</replaceable> | TEXT SEARCH TEMPLATE <replaceable class="PARAMETER">object_name</replaceable> | + TRANSFORM FOR <replaceable>type_name</replaceable> LANGUAGE <replaceable>lang_name</replaceable> | TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable> ON <replaceable class="PARAMETER">table_name</replaceable>| TYPE <replaceable class="PARAMETER">object_name</replaceable> | VIEW <replaceable class="PARAMETER">object_name</replaceable> @@ -217,6 +218,26 @@ <title>Parameters</title> </listitem> </varlistentry> + <varlistentry> + <term><replaceable>type_name</replaceable></term> + + <listitem> + <para> + The name of the data type of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>lang_name</replaceable></term> + + <listitem> + <para> + The name of the language of the transform. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">text</replaceable></term> <listitem> @@ -296,6 +317,7 @@ <title>Examples</title> COMMENT ON TEXT SEARCH DICTIONARY swedish IS 'Snowball stemmer for swedish language'; COMMENT ON TEXT SEARCH PARSER my_parser IS 'Splits text into words'; COMMENT ON TEXT SEARCH TEMPLATE snowball IS 'Snowball stemmer'; +COMMENT ON TRANSFORM FOR hstore LANGUAGE plpythonu IS 'Transform between hstore and Python dict'; COMMENT ON TRIGGER my_trigger ON my_table IS 'Used for RI'; COMMENT ON TYPE complex IS 'Complex number data type'; COMMENT ON VIEW my_view IS 'View of departmental costs'; diff --git a/doc/src/sgml/ref/create_transform.sgml b/doc/src/sgml/ref/create_transform.sgml new file mode 100644 index 0000000..2f10cee --- /dev/null +++ b/doc/src/sgml/ref/create_transform.sgml @@ -0,0 +1,187 @@ +<!-- doc/src/sgml/ref/create_transform.sgml --> + +<refentry id="SQL-CREATETRANSFORM"> + <refmeta> + <refentrytitle>CREATE TRANSFORM</refentrytitle> + <manvolnum>7</manvolnum> + <refmiscinfo>SQL - Language Statements</refmiscinfo> + </refmeta> + + <refnamediv> + <refname>CREATE TRANSFORM</refname> + <refpurpose>define a new transform</refpurpose> + </refnamediv> + + <indexterm zone="sql-createtransform"> + <primary>CREATE TRANSFORM</primary> + </indexterm> + + <refsynopsisdiv> +<synopsis> +CREATE [ OR REPLACE ] TRANSFORM FOR <replaceable>type_name</replaceable> LANGUAGE <replaceable>lang_name</replaceable> ( + FROM SQL WITH FUNCTION <replaceable>from_sql_function_name</replaceable> (<replaceable>argument_type</replaceable> [,...]), + TO SQL WITH FUNCTION <replaceable>to_sql_function_name</replaceable> (<replaceable>argument_type</replaceable> [, ...]) +); +</synopsis> + </refsynopsisdiv> + + <refsect1 id="sql-createtransform-description"> + <title>Description</title> + + <para> + <command>CREATE TRANSFORM</command> defines a new transform. + <command>CREATE OR REPLACE TRANSFORM</command> will either create a new + transform, or replace an existing definition. + </para> + + <para> + A transform specifies how to adapt a data type to a procedural language. + For example, when writing a function in PL/Python using the hstore type, + PL/Python has no prior knowledge how to present hstore values in the Python + environment. Language implementations usually default to using the text + representation, but that is inconvenient when, for example, an associative + array or a list would be more appropriate. A transform specifies two + functions: one <quote>from SQL</quote> function that converts the type from + the SQL environment to the language (In other words, this function will be + invoked on the arguments of a function written in the language.), and one + <quote>to SQL</quote> function that converts the type from the language to + the SQL environment (In other words, this function will be invoked on the + return value of a function written in the language.). It is not necessary + to provide both of these functions. If one is not specified, the + language-specific default behavior will be used if necessary. (To prevent a + transformation in a certain direction from happening at all, you could also + write a transform function that always errors out.) + </para> + + <para> + To be able to create a transform, you must own the type and have + <literal>USAGE</literal> privilege on both the type and the language. + </para> + </refsect1> + + <refsect1> + <title>Parameters</title> + + <variablelist> + <varlistentry> + <term><replaceable>type_name</replaceable></term> + + <listitem> + <para> + The name of the data type of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>lang_name</replaceable></term> + + <listitem> + <para> + The name of the language of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>from_sql_function_name</replaceable>(<replaceable>argument_type</replaceable> [, ...])</term> + + <listitem> + <para> + The name of the function for converting the type from the SQL + environment to the language. It must take one argument of + type <type>internal</type> and return type <type>internal</type>. The + actual argument will be of the type for the transform, and the function + should be coded as if it were, but it is not allowed to declare an + SQL-level function function returning <type>internal</type> without at + least one argument of type <type>internal</type>. The actual return + value will be something specific to the language implementation. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>to_sql_function_name</replaceable>(<replaceable>argument_type</replaceable> [, ...])</term> + + <listitem> + <para> + The name of the function for converting the type from the language to + the SQL environment. It must take one argument of type + <type>internal</type> and return the type that is the type for the + transform. The actual argument value will be something specific to the + language implementation. + </para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1 id="sql-createtransform-notes"> + <title>Notes</title> + + <para> + Use <xref linkend="sql-droptransform"> to remove transforms. + </para> + </refsect1> + + <refsect1 id="sql-createtransform-examples"> + <title>Examples</title> + + <para> + To create a transform for type <type>hstore</type> and language + <literal>plpythonu</literal>, first set up the type and the language: +<programlisting> +CREATE TYPE hstore ...; + +CREATE LANGUAGE plpythonu ...; +</programlisting> + Then create the necessary functions: +<programlisting> +CREATE FUNCTION hstore_to_plpython(val internal) RETURNS internal +LANGUAGE C STRICT IMMUTABLE +AS ...; + +CREATE FUNCTION plpython_to_hstore(val internal) RETURNS hstore +LANGUAGE C STRICT IMMUTABLE +AS ...; +</programlisting> + And finally create the transform to connect them all together: +<programlisting> +CREATE TRANSFORM FOR hstore LANGUAGE plpythonu ( + FROM SQL WITH FUNCTION hstore_to_plpython(internal), + TO SQL WITH FUNCTION plpython_to_hstore(internal) +); +</programlisting> + In practice, these commands would be wrapped up in extensions. + </para> + + <para> + The <filename>contrib</filename> section contains a number of extensions + that provide transforms, which can serve as real-world examples. + </para> + </refsect1> + + <refsect1 id="sql-createtransform-compat"> + <title>Compatibility</title> + + <para> + This form of <command>CREATE TRANSFORM</command> is a + <productname>PostgreSQL</productname> extension. There is a <command>CREATE + TRANSFORM</command> command in the <acronym>SQL</acronym> standard, but it + is for adapting data types to client languages. That usage is not supported + by <productname>PostgreSQL</productname>. + </para> + </refsect1> + + <refsect1 id="sql-createtransform-seealso"> + <title>See Also</title> + + <para> + <xref linkend="sql-createfunction">, + <xref linkend="sql-createlanguage">, + <xref linkend="sql-createtype">, + <xref linkend="sql-droptransform"> + </para> + </refsect1> + +</refentry> diff --git a/doc/src/sgml/ref/drop_transform.sgml b/doc/src/sgml/ref/drop_transform.sgml new file mode 100644 index 0000000..c9b558e --- /dev/null +++ b/doc/src/sgml/ref/drop_transform.sgml @@ -0,0 +1,123 @@ +<!-- doc/src/sgml/ref/drop_transform.sgml --> + +<refentry id="SQL-DROPTRANSFORM"> + <refmeta> + <refentrytitle>DROP TRANSFORM</refentrytitle> + <manvolnum>7</manvolnum> + <refmiscinfo>SQL - Language Statements</refmiscinfo> + </refmeta> + + <refnamediv> + <refname>DROP TRANSFORM</refname> + <refpurpose>remove a transform</refpurpose> + </refnamediv> + + <indexterm zone="sql-droptransform"> + <primary>DROP TRANSFORM</primary> + </indexterm> + + <refsynopsisdiv> +<synopsis> +DROP TRANSFORM [ IF EXISTS ] FOR <replaceable>type_name</replaceable> LANGUAGE <replaceable>lang_name</replaceable> +</synopsis> + </refsynopsisdiv> + + <refsect1 id="sql-droptransform-description"> + <title>Description</title> + + <para> + <command>DROP TRANSFORM</command> removes a previously defined transform. + </para> + + <para> + To be able to drop a transform, you must own the type and the language. + These are the same privileges that are required to create a transform. + </para> + </refsect1> + + <refsect1> + <title>Parameters</title> + + <variablelist> + + <varlistentry> + <term><literal>IF EXISTS</literal></term> + <listitem> + <para> + Do not throw an error if the transform does not exist. A notice is issued + in this case. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>type_name</replaceable></term> + + <listitem> + <para> + The name of the data type of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable>lang_name</replaceable></term> + + <listitem> + <para> + The name of the language of the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>CASCADE</literal></term> + <listitem> + <para> + Automatically drop objects that depend on the transform. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><literal>RESTRICT</literal></term> + <listitem> + <para> + Refuse to drop the transform if any objects depend on it. This is the + default. + </para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1 id="sql-droptransform-examples"> + <title>Examples</title> + + <para> + To drop the transform for type <type>hstore</type> and language + <literal>plpythonu</literal>: +<programlisting> +DROP TRANSFORM FOR hstore LANGUAGE plpythonu; +</programlisting></para> + </refsect1> + + <refsect1 id="sql-droptransform-compat"> + <title>Compatibility</title> + + <para> + This form of <command>DROP TRANSFORM</command> is a + <productname>PostgreSQL</productname> extension. See <xref + linkend="sql-createtransform"> for details. + </para> + </refsect1> + + <refsect1> + <title>See Also</title> + + <simplelist type="inline"> + <member><xref linkend="sql-createtransform"></member> + </simplelist> + </refsect1> + +</refentry> diff --git a/doc/src/sgml/reference.sgml b/doc/src/sgml/reference.sgml index 14e217a..0aac876 100644 --- a/doc/src/sgml/reference.sgml +++ b/doc/src/sgml/reference.sgml @@ -108,6 +108,7 @@ <title>SQL Commands</title> &createTSDictionary; &createTSParser; &createTSTemplate; + &createTransform; &createTrigger; &createType; &createUser; @@ -148,6 +149,7 @@ <title>SQL Commands</title> &dropTSDictionary; &dropTSParser; &dropTSTemplate; + &dropTransform; &dropTrigger; &dropType; &dropUser; diff --git a/src/Makefile.shlib b/src/Makefile.shlib index 2a0c7a9..3ab9b82 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) diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile index c4d3f3c..f02ea60 100644 --- a/src/backend/catalog/Makefile +++ b/src/backend/catalog/Makefile @@ -41,6 +41,7 @@ POSTGRES_BKI_SRCS = $(addprefix $(top_srcdir)/src/include/catalog/,\ pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \ pg_foreign_table.h \ pg_default_acl.h pg_seclabel.h pg_shseclabel.h pg_collation.h pg_range.h \ + pg_transform.h \ toasting.h indexing.h \ ) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 69171f8..52aae48 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -46,6 +46,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_rewrite.h" #include "catalog/pg_tablespace.h" +#include "catalog/pg_transform.h" #include "catalog/pg_trigger.h" #include "catalog/pg_ts_config.h" #include "catalog/pg_ts_dict.h" @@ -1249,6 +1250,10 @@ static bool stack_address_present_add_flags(const ObjectAddress *object, RemoveEventTriggerById(object->objectId); break; + case OCLASS_TRANSFORM: + DropTransformById(object->objectId); + break; + default: elog(ERROR, "unrecognized object class: %u", object->classId); @@ -2308,6 +2313,9 @@ static bool stack_address_present_add_flags(const ObjectAddress *object, case EventTriggerRelationId: return OCLASS_EVENT_TRIGGER; + + case TransformRelationId: + return OCLASS_TRANSFORM; } /* shouldn't get here */ diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 215eaf5..dd267c1 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -44,6 +44,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_rewrite.h" #include "catalog/pg_tablespace.h" +#include "catalog/pg_transform.h" #include "catalog/pg_trigger.h" #include "catalog/pg_ts_config.h" #include "catalog/pg_ts_dict.h" @@ -332,6 +333,12 @@ true }, { + TransformRelationId, + TransformOidIndexId, + TRFOID, + InvalidAttrNumber + }, + { TriggerRelationId, TriggerOidIndexId, -1, @@ -589,6 +596,29 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid, address.objectSubId = 0; } break; + case OBJECT_TRANSFORM: + { + TypeName *typename = (TypeName *) linitial(objname); + char *langname = (char *) linitial(objargs); + Oid typeid = typenameTypeId(NULL, typename); + Oid langid; + HeapTuple tuple; + + tuple = SearchSysCache1(LANGNAME, PointerGetDatum(langname)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("language \"%s\" does not exist", langname))); + + langid = HeapTupleGetOid(tuple); + ReleaseSysCache(tuple); + + address.classId = TransformRelationId; + address.objectId = + get_transform_oid(typeid, langid, missing_ok); + address.objectSubId = 0; + } + break; case OBJECT_TSPARSER: address.classId = TSParserRelationId; address.objectId = get_ts_parser_oid(objname, missing_ok); @@ -1234,6 +1264,16 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid, format_type_be(targettypeid)))); } break; + case OBJECT_TRANSFORM: + { + TypeName *typename = (TypeName *) linitial(objname); + Oid typeid = typenameTypeId(NULL, typename); + + if (!pg_type_ownercheck(typeid, roleid)) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TYPE, + format_type_be(typeid)); + } + break; case OBJECT_TABLESPACE: if (!pg_tablespace_ownercheck(address.objectId, roleid)) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TABLESPACE, @@ -1667,19 +1707,10 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid, } case OCLASS_LANGUAGE: - { - HeapTuple langTup; + appendStringInfo(&buffer, _("language %s"), + get_language_name(object->objectId, false)); + break; - langTup = SearchSysCache1(LANGOID, - ObjectIdGetDatum(object->objectId)); - if (!HeapTupleIsValid(langTup)) - elog(ERROR, "cache lookup failed for language %u", - object->objectId); - appendStringInfo(&buffer, _("language %s"), - NameStr(((Form_pg_language) GETSTRUCT(langTup))->lanname)); - ReleaseSysCache(langTup); - break; - } case OCLASS_LARGEOBJECT: appendStringInfo(&buffer, _("large object %u"), object->objectId); @@ -1867,6 +1898,27 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid, break; } + 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; + } + case OCLASS_TRIGGER: { Relation trigDesc; diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 2a98ca9..96aabb1 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -23,6 +23,7 @@ #include "catalog/pg_namespace.h" #include "catalog/pg_proc.h" #include "catalog/pg_proc_fn.h" +#include "catalog/pg_transform.h" #include "catalog/pg_type.h" #include "executor/functions.h" #include "funcapi.h" @@ -116,6 +117,7 @@ static bool match_prosrc_to_literal(const char *prosrc, const char *literal, ObjectAddress myself, referenced; int i; + Oid trfid; /* * sanity checks @@ -624,6 +626,15 @@ static bool match_prosrc_to_literal(const char *prosrc, const char *literal, referenced.objectSubId = 0; recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + /* dependency on transform used by return type, if any */ + if ((trfid = get_transform(returnType, languageObjectId))) + { + referenced.classId = TransformRelationId; + referenced.objectId = trfid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + } + /* dependency on parameter types */ for (i = 0; i < allParamCount; i++) { @@ -631,6 +642,15 @@ static bool match_prosrc_to_literal(const char *prosrc, const char *literal, referenced.objectId = allParams[i]; referenced.objectSubId = 0; recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + + /* dependency on transform used by parameter type, if any */ + if ((trfid = get_transform(allParams[i], languageObjectId))) + { + referenced.classId = TransformRelationId; + referenced.objectId = trfid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + } } /* dependency on parameter default expressions */ diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c index b32ad3a..c63e07b 100644 --- a/src/backend/commands/dropcmds.c +++ b/src/backend/commands/dropcmds.c @@ -200,6 +200,12 @@ static void does_not_exist_skipping(ObjectType objtype, args = format_type_be(typenameTypeId(NULL, (TypeName *) linitial(objargs))); break; + case OBJECT_TRANSFORM: + msg = gettext_noop("transform for type %s language %s does not exist, skipping"); + name = format_type_be(typenameTypeId(NULL, + (TypeName *) linitial(objname))); + args = (char *) linitial(objargs); + break; case OBJECT_TRIGGER: msg = gettext_noop("trigger \"%s\" for table \"%s\" does not exist, skipping"); name = strVal(llast(objname)); diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index 328e2a8..7a53d56 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -92,6 +92,7 @@ {"SERVER", true}, {"TABLE", true}, {"TABLESPACE", false}, + {"TRANSFORM", true}, {"TRIGGER", true}, {"TEXT SEARCH CONFIGURATION", true}, {"TEXT SEARCH DICTIONARY", true}, @@ -937,6 +938,7 @@ static Oid insert_event_trigger_tuple(char *trigname, char *eventname, case OBJECT_SCHEMA: case OBJECT_SEQUENCE: case OBJECT_TABLE: + case OBJECT_TRANSFORM: case OBJECT_TRIGGER: case OBJECT_TSCONFIGURATION: case OBJECT_TSDICTIONARY: @@ -983,6 +985,7 @@ static Oid insert_event_trigger_tuple(char *trigname, char *eventname, case OCLASS_REWRITE: case OCLASS_TRIGGER: case OCLASS_SCHEMA: + case OCLASS_TRANSFORM: case OCLASS_TSPARSER: case OCLASS_TSDICT: case OCLASS_TSTEMPLATE: diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index c776758..17ba792 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -45,6 +45,7 @@ #include "catalog/pg_namespace.h" #include "catalog/pg_proc.h" #include "catalog/pg_proc_fn.h" +#include "catalog/pg_transform.h" #include "catalog/pg_type.h" #include "catalog/pg_type_fn.h" #include "commands/alter.h" @@ -1618,6 +1619,298 @@ heap_close(relation, RowExclusiveLock); } + +static void +check_transform_function(Form_pg_proc procstruct) +{ + if (procstruct->provolatile == PROVOLATILE_VOLATILE) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("transform function must not be volatile"))); + if (procstruct->proisagg) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("transform function must not be an aggregate function"))); + if (procstruct->proiswindow) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("transform function must not be a window function"))); + if (procstruct->proretset) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("transform function must not return a set"))); + if (procstruct->pronargs != 1) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("transform function must take one argument"))); + if (procstruct->proargtypes.values[0] != INTERNALOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("first argument of transform function must be type \"internal\""))); +} + + +/* + * CREATE TRANSFORM + */ +Oid +CreateTransform(CreateTransformStmt *stmt) +{ + Oid typeid; + char typtype; + Oid langid; + Oid fromsqlfuncid; + Oid tosqlfuncid; + AclResult aclresult; + Form_pg_proc procstruct; + Datum values[Natts_pg_transform]; + bool nulls[Natts_pg_transform]; + bool replaces[Natts_pg_transform]; + Oid transformid; + HeapTuple tuple; + HeapTuple newtuple; + Relation relation; + ObjectAddress myself, + referenced; + bool is_replace; + + /* + * Get the type + */ + typeid = typenameTypeId(NULL, stmt->type_name); + typtype = get_typtype(typeid); + + if (typtype == TYPTYPE_PSEUDO) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("data type %s is a pseudo-type", + TypeNameToString(stmt->type_name)))); + + if (typtype == TYPTYPE_DOMAIN) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("data type %s is a domain", + TypeNameToString(stmt->type_name)))); + + if (!pg_type_ownercheck(typeid, GetUserId())) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be owner of type %s", + format_type_be(typeid)))); + + aclresult = pg_type_aclcheck(typeid, GetUserId(), ACL_USAGE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_TYPE, + format_type_be(typeid)); + + /* + * Get the language + */ + tuple = SearchSysCache1(LANGNAME, PointerGetDatum(stmt->lang)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("language \"%s\" does not exist", stmt->lang))); + + langid = HeapTupleGetOid(tuple); + ReleaseSysCache(tuple); + + aclresult = pg_language_aclcheck(langid, GetUserId(), ACL_USAGE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_LANGUAGE, stmt->lang); + + /* + * Get the functions + */ + if (stmt->fromsql) + { + fromsqlfuncid = LookupFuncNameTypeNames(stmt->fromsql->funcname, stmt->fromsql->funcargs, false); + + 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); + } + else + fromsqlfuncid = InvalidOid; + + if (stmt->tosql) + { + tosqlfuncid = LookupFuncNameTypeNames(stmt->tosql->funcname, stmt->tosql->funcargs, false); + + aclresult = pg_proc_aclcheck(tosqlfuncid, GetUserId(), ACL_EXECUTE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_PROC, NameListToString(stmt->tosql->funcname)); + + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(tosqlfuncid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for function %u", tosqlfuncid); + procstruct = (Form_pg_proc) GETSTRUCT(tuple); + if (procstruct->prorettype != typeid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("return data type of TO SQL function must be the transform data type"))); + check_transform_function(procstruct); + ReleaseSysCache(tuple); + } + else + tosqlfuncid = InvalidOid; + + /* + * Ready to go + */ + values[Anum_pg_transform_trftype - 1] = ObjectIdGetDatum(typeid); + values[Anum_pg_transform_trflang - 1] = ObjectIdGetDatum(langid); + values[Anum_pg_transform_trffromsql - 1] = ObjectIdGetDatum(fromsqlfuncid); + values[Anum_pg_transform_trftosql - 1] = ObjectIdGetDatum(tosqlfuncid); + + MemSet(nulls, false, sizeof(nulls)); + + relation = heap_open(TransformRelationId, RowExclusiveLock); + + tuple = SearchSysCache2(TRFTYPELANG, + ObjectIdGetDatum(typeid), + ObjectIdGetDatum(langid)); + if (HeapTupleIsValid(tuple)) + { + if (!stmt->replace) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("transform for type %s language %s already exists", + format_type_be(typeid), + stmt->lang))); + + MemSet(replaces, false, sizeof(replaces)); + replaces[Anum_pg_transform_trffromsql - 1] = true; + replaces[Anum_pg_transform_trftosql - 1] = true; + + newtuple = heap_modify_tuple(tuple, RelationGetDescr(relation), values, nulls, replaces); + simple_heap_update(relation, &newtuple->t_self, newtuple); + + transformid = HeapTupleGetOid(tuple); + ReleaseSysCache(tuple); + is_replace = true; + } + else + { + newtuple = heap_form_tuple(RelationGetDescr(relation), values, nulls); + transformid = simple_heap_insert(relation, newtuple); + is_replace = false; + } + + CatalogUpdateIndexes(relation, newtuple); + + if (is_replace) + deleteDependencyRecordsFor(TransformRelationId, transformid, true); + + /* make dependency entries */ + myself.classId = TransformRelationId; + myself.objectId = transformid; + myself.objectSubId = 0; + + /* dependency on language */ + referenced.classId = LanguageRelationId; + referenced.objectId = langid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + + /* dependency on type */ + referenced.classId = TypeRelationId; + referenced.objectId = typeid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + + /* dependencies on functions */ + if (OidIsValid(fromsqlfuncid)) + { + referenced.classId = ProcedureRelationId; + referenced.objectId = fromsqlfuncid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + } + if (OidIsValid(tosqlfuncid)) + { + referenced.classId = ProcedureRelationId; + referenced.objectId = tosqlfuncid; + referenced.objectSubId = 0; + recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + } + + /* dependency on extension */ + recordDependencyOnCurrentExtension(&myself, is_replace); + + /* Post creation hook for new transform */ + InvokeObjectPostCreateHook(TransformRelationId, transformid, 0); + + heap_freetuple(newtuple); + + heap_close(relation, RowExclusiveLock); + + return transformid; +} + + +/* + * get_transform_oid - given type OID and language OID, look up a transform OID + * + * If missing_ok is false, throw an error if the transform is not found. If + * true, just return InvalidOid. + */ +Oid +get_transform_oid(Oid typeid, Oid langid, bool missing_ok) +{ + Oid oid; + + oid = GetSysCacheOid2(TRFTYPELANG, + ObjectIdGetDatum(typeid), + ObjectIdGetDatum(langid)); + if (!OidIsValid(oid) && !missing_ok) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("transform for type %s language \"%s\" does not exist", + format_type_be(typeid), + get_language_name(langid, false)))); + return oid; +} + + +void +DropTransformById(Oid transformOid) +{ + Relation relation; + ScanKeyData scankey; + SysScanDesc scan; + HeapTuple tuple; + + relation = heap_open(TransformRelationId, RowExclusiveLock); + + ScanKeyInit(&scankey, + ObjectIdAttributeNumber, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(transformOid)); + scan = systable_beginscan(relation, TransformOidIndexId, true, + SnapshotNow, 1, &scankey); + + tuple = systable_getnext(scan); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for transform %u", transformOid); + simple_heap_delete(relation, &tuple->t_self); + + systable_endscan(scan); + heap_close(relation, RowExclusiveLock); +} + + /* * Subroutine for ALTER FUNCTION/AGGREGATE SET SCHEMA/RENAME * diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index b5b8d63..4dd60ea 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3476,6 +3476,20 @@ return newnode; } +static CreateTransformStmt * +_copyCreateTransformStmt(const CreateTransformStmt *from) +{ + CreateTransformStmt *newnode = makeNode(CreateTransformStmt); + + COPY_SCALAR_FIELD(replace); + COPY_NODE_FIELD(type_name); + COPY_STRING_FIELD(lang); + COPY_NODE_FIELD(fromsql); + COPY_NODE_FIELD(tosql); + + return newnode; +} + static CreateTrigStmt * _copyCreateTrigStmt(const CreateTrigStmt *from) { @@ -4377,6 +4391,9 @@ case T_CreateForeignTableStmt: retval = _copyCreateForeignTableStmt(from); break; + case T_CreateTransformStmt: + retval = _copyCreateTransformStmt(from); + break; case T_CreateTrigStmt: retval = _copyCreateTrigStmt(from); break; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 3f96595..44f4b8f 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1721,6 +1721,18 @@ } static bool +_equalCreateTransformStmt(const CreateTransformStmt *a, const CreateTransformStmt *b) +{ + COMPARE_SCALAR_FIELD(replace); + COMPARE_NODE_FIELD(type_name); + COMPARE_STRING_FIELD(lang); + COMPARE_NODE_FIELD(fromsql); + COMPARE_NODE_FIELD(tosql); + + return true; +} + +static bool _equalCreateTrigStmt(const CreateTrigStmt *a, const CreateTrigStmt *b) { COMPARE_STRING_FIELD(trigname); @@ -2847,6 +2859,9 @@ case T_CreateForeignTableStmt: retval = _equalCreateForeignTableStmt(a, b); break; + case T_CreateTransformStmt: + retval = _equalCreateTransformStmt(a, b); + break; case T_CreateTrigStmt: retval = _equalCreateTrigStmt(a, b); break; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 5094226..afc33e3 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -227,12 +227,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); CreateOpFamilyStmt AlterOpFamilyStmt CreatePLangStmt CreateSchemaStmt CreateSeqStmt CreateStmt CreateTableSpaceStmt CreateFdwStmt CreateForeignServerStmt CreateForeignTableStmt - CreateAssertStmt CreateTrigStmt CreateEventTrigStmt + CreateAssertStmt CreateTransformStmt CreateTrigStmt CreateEventTrigStmt CreateUserStmt CreateUserMappingStmt CreateRoleStmt CreatedbStmt DeclareCursorStmt DefineStmt DeleteStmt DiscardStmt DoStmt DropGroupStmt DropOpClassStmt DropOpFamilyStmt DropPLangStmt DropStmt DropAssertStmt DropTrigStmt DropRuleStmt DropCastStmt DropRoleStmt - DropUserStmt DropdbStmt DropTableSpaceStmt DropFdwStmt + DropUserStmt DropdbStmt DropTableSpaceStmt DropFdwStmt DropTransformStmt DropForeignServerStmt DropUserMappingStmt ExplainStmt FetchStmt GrantStmt GrantRoleStmt IndexStmt InsertStmt ListenStmt LoadStmt LockStmt NotifyStmt ExplainableStmt PreparableStmt @@ -344,6 +344,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); opt_enum_val_list enum_val_list table_func_column_list create_generic_options alter_generic_options relation_expr_list dostmt_opt_list + transform_element_list %type <list> opt_fdw_options fdw_options %type <defelt> fdw_option @@ -578,12 +579,12 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); SAVEPOINT SCHEMA SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES SERIALIZABLE SERVER SESSION SESSION_USER SET SETOF SHARE - SHOW SIMILAR SIMPLE SMALLINT SNAPSHOT SOME STABLE STANDALONE_P START + SHOW SIMILAR SIMPLE SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P START STATEMENT STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P SUBSTRING SYMMETRIC SYSID SYSTEM_P TABLE TABLES TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIME TIMESTAMP - TO TRAILING TRANSACTION TREAT TRIGGER TRIM TRUE_P + TO TRAILING TRANSACTION TRANSFORM TREAT TRIGGER TRIM TRUE_P TRUNCATE TRUSTED TYPE_P TYPES_P UNBOUNDED UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN UNLISTEN UNLOGGED @@ -753,6 +754,7 @@ stmt : | CreateSeqStmt | CreateStmt | CreateTableSpaceStmt + | CreateTransformStmt | CreateTrigStmt | CreateEventTrigStmt | CreateRoleStmt @@ -777,6 +779,7 @@ stmt : | DropRuleStmt | DropStmt | DropTableSpaceStmt + | DropTransformStmt | DropTrigStmt | DropRoleStmt | DropUserStmt @@ -3847,6 +3850,16 @@ AlterExtensionContentsStmt: n->objname = list_make1(makeString($6)); $$ = (Node *)n; } + | ALTER EXTENSION name add_drop TRANSFORM FOR Typename LANGUAGE name + { + AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); + n->extname = $3; + n->action = $4; + n->objtype = OBJECT_TRANSFORM; + n->objname = list_make1($7); + n->objargs = list_make1($9); + $$ = (Node *)n; + } | ALTER EXTENSION name add_drop TYPE_P any_name { AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); @@ -5274,6 +5287,15 @@ CommentStmt: n->comment = $6; $$ = (Node *) n; } + | COMMENT ON TRANSFORM FOR Typename LANGUAGE name IS comment_text + { + CommentStmt *n = makeNode(CommentStmt); + n->objtype = OBJECT_TRANSFORM; + n->objname = list_make1($5); + n->objargs = list_make1($7); + n->comment = $9; + $$ = (Node *) n; + } | COMMENT ON TRIGGER name ON any_name IS comment_text { CommentStmt *n = makeNode(CommentStmt); @@ -6744,6 +6766,56 @@ opt_if_exists: IF_P EXISTS { $$ = TRUE; } /***************************************************************************** * + * CREATE TRANSFORM / DROP TRANSFORM + * + *****************************************************************************/ + +CreateTransformStmt: CREATE opt_or_replace TRANSFORM FOR Typename LANGUAGE name '(' transform_element_list ')' + { + CreateTransformStmt *n = makeNode(CreateTransformStmt); + n->replace = $2; + n->type_name = $5; + n->lang = $7; + n->fromsql = linitial($9); + n->tosql = lsecond($9); + $$ = (Node *)n; + } + ; + +transform_element_list: FROM SQL_P WITH FUNCTION function_with_argtypes ',' TO SQL_P WITH FUNCTION function_with_argtypes + { + $$ = list_make2($5, $11); + } + | TO SQL_P WITH FUNCTION function_with_argtypes ',' FROM SQL_P WITH FUNCTION function_with_argtypes + { + $$ = list_make2($11, $5); + } + | FROM SQL_P WITH FUNCTION function_with_argtypes + { + $$ = list_make2($5, NULL); + } + | TO SQL_P WITH FUNCTION function_with_argtypes + { + $$ = list_make2(NULL, $5); + } + ; + + +DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_drop_behavior + { + DropStmt *n = makeNode(DropStmt); + n->removeType = OBJECT_TRANSFORM; + n->objects = list_make1(list_make1($5)); + n->arguments = list_make1(list_make1($7)); + n->behavior = $8; + n->missing_ok = $3; + $$ = (Node *)n; + } + ; + + +/***************************************************************************** + * * QUERY: * * REINDEX type <name> [FORCE] @@ -12851,6 +12923,7 @@ unreserved_keyword: | SHOW | SIMPLE | SNAPSHOT + | SQL_P | STABLE | STANDALONE_P | START @@ -12870,6 +12943,7 @@ unreserved_keyword: | TEMPORARY | TEXT_P | TRANSACTION + | TRANSFORM | TRIGGER | TRUNCATE | TRUSTED diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index ae7d195..7c99938 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -306,6 +306,11 @@ static Node *ParseComplexProjection(ParseState *pstate, char *funcname, parser_errposition(pstate, location))); } + if (rettype == INTERNALOID) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot call function returning type \"internal\""))); + /* * If there are default arguments, we have to include their types in * actual_arg_types for the purpose of checking generic type consistency. diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index c940897..3466b9c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -214,6 +214,7 @@ static void ProcessUtilitySlow(Node *parsetree, case T_CreateTableAsStmt: case T_RefreshMatViewStmt: case T_CreateTableSpaceStmt: + case T_CreateTransformStmt: case T_CreateTrigStmt: case T_CompositeTypeStmt: case T_CreateEnumStmt: @@ -1290,6 +1291,10 @@ static void ProcessUtilitySlow(Node *parsetree, DefineOpFamily((CreateOpFamilyStmt *) parsetree); break; + case T_CreateTransformStmt: + CreateTransform((CreateTransformStmt *) parsetree); + break; + case T_AlterOpFamilyStmt: AlterOpFamily((AlterOpFamilyStmt *) parsetree); break; @@ -1946,6 +1951,9 @@ static void ProcessUtilitySlow(Node *parsetree, case OBJECT_OPFAMILY: tag = "DROP OPERATOR FAMILY"; break; + case OBJECT_TRANSFORM: + tag = "DROP TRANSFORM"; + break; default: tag = "???"; } @@ -2194,6 +2202,10 @@ static void ProcessUtilitySlow(Node *parsetree, } break; + case T_CreateTransformStmt: + tag = "CREATE TRANSFORM"; + break; + case T_CreateTrigStmt: tag = "CREATE TRIGGER"; break; @@ -2809,6 +2821,10 @@ static void ProcessUtilitySlow(Node *parsetree, lev = LOGSTMT_DDL; break; + case T_CreateTransformStmt: + lev = LOGSTMT_DDL; + break; + case T_AlterOpFamilyStmt: lev = LOGSTMT_DDL; break; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index a1ed781..f50f374 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1869,9 +1869,7 @@ static char *generate_function_name(Oid funcid, int nargs, StringInfoData buf; StringInfoData dq; HeapTuple proctup; - HeapTuple langtup; Form_pg_proc proc; - Form_pg_language lang; Datum tmp; bool isnull; const char *prosrc; @@ -1894,12 +1892,6 @@ static char *generate_function_name(Oid funcid, int nargs, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is an aggregate function", name))); - /* Need its pg_language tuple for the language name */ - langtup = SearchSysCache1(LANGOID, ObjectIdGetDatum(proc->prolang)); - if (!HeapTupleIsValid(langtup)) - elog(ERROR, "cache lookup failed for language %u", proc->prolang); - lang = (Form_pg_language) GETSTRUCT(langtup); - /* * We always qualify the function name, to ensure the right function gets * replaced. @@ -1911,7 +1903,7 @@ static char *generate_function_name(Oid funcid, int nargs, appendStringInfoString(&buf, ")\n RETURNS "); print_function_rettype(&buf, proctup); appendStringInfo(&buf, "\n LANGUAGE %s\n", - quote_identifier(NameStr(lang->lanname))); + quote_identifier(get_language_name(proc->prolang, false))); /* Emit some miscellaneous options on one line */ oldlen = buf.len; @@ -2031,7 +2023,6 @@ static char *generate_function_name(Oid funcid, int nargs, appendStringInfoString(&buf, "\n"); - ReleaseSysCache(langtup); ReleaseSysCache(proctup); PG_RETURN_TEXT_P(string_to_text(buf.data)); diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 5865962..fd08a7d 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -23,12 +23,14 @@ #include "catalog/pg_amproc.h" #include "catalog/pg_collation.h" #include "catalog/pg_constraint.h" +#include "catalog/pg_language.h" #include "catalog/pg_namespace.h" #include "catalog/pg_opclass.h" #include "catalog/pg_operator.h" #include "catalog/pg_proc.h" #include "catalog/pg_range.h" #include "catalog/pg_statistic.h" +#include "catalog/pg_transform.h" #include "catalog/pg_type.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -1032,6 +1034,30 @@ return NULL; } +/* ---------- LANGUAGE CACHE ---------- */ + +char * +get_language_name(Oid langoid, bool missing_ok) +{ + HeapTuple tp; + + tp = SearchSysCache1(LANGOID, ObjectIdGetDatum(langoid)); + if (HeapTupleIsValid(tp)) + { + Form_pg_language lantup = (Form_pg_language) GETSTRUCT(tp); + char *result; + + result = pstrdup(NameStr(lantup->lanname)); + ReleaseSysCache(tp); + return result; + } + + if (!missing_ok) + elog(ERROR, "cache lookup failed for language %u", + langoid); + return NULL; +} + /* ---------- OPCLASS CACHE ---------- */ /* @@ -1779,6 +1805,63 @@ } +/* ---------- TRANSFORM CACHE ---------- */ + +Oid +get_transform(Oid typid, Oid langid) +{ + HeapTuple tup; + + tup = SearchSysCache2(TRFTYPELANG, typid, langid); + if (HeapTupleIsValid(tup)) + { + Oid id; + + id = HeapTupleGetOid(tup); + ReleaseSysCache(tup); + return id; + } + else + return InvalidOid; +} + +Oid +get_transform_fromsql(Oid typid, Oid langid) +{ + HeapTuple tup; + + tup = SearchSysCache2(TRFTYPELANG, typid, langid); + if (HeapTupleIsValid(tup)) + { + Oid funcid; + + funcid = ((Form_pg_transform) GETSTRUCT(tup))->trffromsql; + ReleaseSysCache(tup); + return funcid; + } + else + return InvalidOid; +} + +Oid +get_transform_tosql(Oid typid, Oid langid) +{ + HeapTuple tup; + + tup = SearchSysCache2(TRFTYPELANG, typid, langid); + if (HeapTupleIsValid(tup)) + { + Oid funcid; + + funcid = ((Form_pg_transform) GETSTRUCT(tup))->trftosql; + ReleaseSysCache(tup); + return funcid; + } + else + return InvalidOid; +} + + /* ---------- TYPE CACHE ---------- */ /* diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c index ecb0f96..6e6cef4 100644 --- a/src/backend/utils/cache/syscache.c +++ b/src/backend/utils/cache/syscache.c @@ -49,6 +49,7 @@ #include "catalog/pg_rewrite.h" #include "catalog/pg_statistic.h" #include "catalog/pg_tablespace.h" +#include "catalog/pg_transform.h" #include "catalog/pg_ts_config.h" #include "catalog/pg_ts_config_map.h" #include "catalog/pg_ts_dict.h" @@ -646,6 +647,28 @@ struct cachedesc }, 16 }, + {TransformRelationId, /* TRFOID */ + TransformOidIndexId, + 1, + { + ObjectIdAttributeNumber, + 0, + 0, + 0, + }, + 16 + }, + {TransformRelationId, /* TRFTYPELANG */ + TransformTypeLangIndexId, + 2, + { + Anum_pg_transform_trftype, + Anum_pg_transform_trflang, + 0, + 0, + }, + 16 + }, {TSConfigMapRelationId, /* TSCONFIGMAP */ TSConfigMapIndexId, 3, diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 58322dc..212d4d7 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -89,6 +89,7 @@ static void findParentsByOid(TableInfo *self, int numRules; int numProcLangs; int numCasts; + int numTransforms; int numOpclasses; int numOpfamilies; int numConversions; @@ -199,6 +200,10 @@ static void findParentsByOid(TableInfo *self, getCasts(fout, &numCasts); if (g_verbose) + write_msg(NULL, "reading transforms\n"); + getTransforms(fout, &numTransforms); + + if (g_verbose) write_msg(NULL, "reading table inheritance information\n"); inhinfo = getInherits(fout, &numInherits); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ec956ad..c4da9e5 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -182,6 +182,7 @@ static int findSecLabels(Archive *fout, Oid classoid, Oid objoid, static void dumpProcLang(Archive *fout, ProcLangInfo *plang); static void dumpFunc(Archive *fout, FuncInfo *finfo); static void dumpCast(Archive *fout, CastInfo *cast); +static void dumpTransform(Archive *fout, TransformInfo *transform); static void dumpOpr(Archive *fout, OprInfo *oprinfo); static void dumpOpclass(Archive *fout, OpclassInfo *opcinfo); static void dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo); @@ -6097,6 +6098,110 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, return castinfo; } +static char * +get_language_name(Archive *fout, Oid langid) +{ + PQExpBuffer query; + PGresult *res; + char *lanname; + + query = createPQExpBuffer(); + appendPQExpBuffer(query, "SELECT lanname FROM pg_language WHERE oid = %u", langid); + res = ExecuteSqlQueryForSingleRow(fout, query->data); + lanname = pg_strdup(fmtId(PQgetvalue(res, 0, 0))); + destroyPQExpBuffer(query); + PQclear(res); + + return lanname; +} + +/* + * getTransforms + * get basic information about every transform in the system + * + * numTransforms is set to the number of transforms read in + */ +TransformInfo * +getTransforms(Archive *fout, int *numTransforms) +{ + PGresult *res; + int ntups; + int i; + PQExpBuffer query = createPQExpBuffer(); + TransformInfo *transforminfo; + int i_tableoid; + int i_oid; + int i_trftype; + int i_trflang; + int i_trffromsql; + int i_trftosql; + + /* Transforms didn't exist pre-9.4 */ + if (fout->remoteVersion < 90400) + { + *numTransforms = 0; + return NULL; + } + + /* Make sure we are in proper schema */ + selectSourceSchema(fout, "pg_catalog"); + + appendPQExpBuffer(query, "SELECT tableoid, oid, " + "trftype, trflang, trffromsql::oid, trftosql::oid " + "FROM pg_transform " + "ORDER BY 3,4"); + + res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + + ntups = PQntuples(res); + + *numTransforms = ntups; + + transforminfo = (TransformInfo *) pg_malloc(ntups * sizeof(TransformInfo)); + + i_tableoid = PQfnumber(res, "tableoid"); + i_oid = PQfnumber(res, "oid"); + i_trftype = PQfnumber(res, "trftype"); + i_trflang = PQfnumber(res, "trflang"); + i_trffromsql = PQfnumber(res, "trffromsql"); + i_trftosql = PQfnumber(res, "trftosql"); + + for (i = 0; i < ntups; i++) + { + PQExpBufferData namebuf; + TypeInfo *typeInfo; + char *lanname; + + transforminfo[i].dobj.objType = DO_TRANSFORM; + transforminfo[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid)); + transforminfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid)); + AssignDumpId(&transforminfo[i].dobj); + transforminfo[i].trftype = atooid(PQgetvalue(res, i, i_trftype)); + transforminfo[i].trflang = atooid(PQgetvalue(res, i, i_trflang)); + transforminfo[i].trffromsql = atooid(PQgetvalue(res, i, i_trffromsql)); + transforminfo[i].trftosql = atooid(PQgetvalue(res, i, i_trftosql)); + + /* + * Try to name transform as concatenation of type and language name. + * This is only used for purposes of sorting. If we fail to find + * either, the name will be an empty string. + */ + initPQExpBuffer(&namebuf); + typeInfo = findTypeByOid(transforminfo[i].trftype); + lanname = get_language_name(fout, transforminfo[i].trflang); + if (typeInfo && lanname) + appendPQExpBuffer(&namebuf, "%s %s", + typeInfo->dobj.name, lanname); + transforminfo[i].dobj.name = namebuf.data; + } + + PQclear(res); + + destroyPQExpBuffer(query); + + return transforminfo; +} + /* * getTableAttrs - * for each interesting table, read info about its attributes @@ -7707,6 +7812,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, case DO_CAST: dumpCast(fout, (CastInfo *) dobj); break; + case DO_TRANSFORM: + dumpTransform(fout, (TransformInfo *) dobj); + break; case DO_TABLE_DATA: if (((TableDataInfo *) dobj)->tdtable->relkind == RELKIND_SEQUENCE) dumpSequenceData(fout, (TableDataInfo *) dobj); @@ -10098,6 +10206,127 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, } /* + * Dump a transform + */ +static void +dumpTransform(Archive *fout, TransformInfo *transform) +{ + PQExpBuffer defqry; + PQExpBuffer delqry; + PQExpBuffer labelq; + FuncInfo *fromsqlFuncInfo = NULL; + FuncInfo *tosqlFuncInfo = NULL; + char *lanname; + + /* Skip if not to be dumped */ + if (!transform->dobj.dump || dataOnly) + return; + + /* Cannot dump if we don't have the transform functions' info */ + if (OidIsValid(transform->trffromsql)) + { + fromsqlFuncInfo = findFuncByOid(transform->trffromsql); + if (fromsqlFuncInfo == NULL) + return; + } + if (OidIsValid(transform->trftosql)) + { + tosqlFuncInfo = findFuncByOid(transform->trftosql); + if (tosqlFuncInfo == NULL) + return; + } + + /* Make sure we are in proper schema (needed for getFormattedTypeName) */ + selectSourceSchema(fout, "pg_catalog"); + + defqry = createPQExpBuffer(); + delqry = createPQExpBuffer(); + labelq = createPQExpBuffer(); + + lanname = get_language_name(fout, transform->trflang); + + appendPQExpBuffer(delqry, "DROP TRANSFORM FOR %s LANGUAGE %s;\n", + getFormattedTypeName(fout, transform->trftype, zeroAsNone), + lanname); + + appendPQExpBuffer(defqry, "CREATE TRANSFORM FOR %s LANGUAGE %s (", + getFormattedTypeName(fout, transform->trftype, zeroAsNone), + lanname); + + if (!transform->trffromsql && !transform->trftosql) + write_msg(NULL, "WARNING: bogus transform definition, at least one of trffromsql and trftosql should be nonzero\n"); + + if (transform->trffromsql) + { + if (fromsqlFuncInfo) + { + char *fsig = format_function_signature(fout, fromsqlFuncInfo, true); + + /* + * Always qualify the function name, in case it is not in + * pg_catalog schema (format_function_signature won't qualify + * it). + */ + appendPQExpBuffer(defqry, "FROM SQL WITH FUNCTION %s.%s", + fmtId(fromsqlFuncInfo->dobj.namespace->dobj.name), fsig); + free(fsig); + } + else + write_msg(NULL, "WARNING: bogus value in pg_transform.trffromsql field\n"); + } + + if (transform->trftosql) + { + if (transform->trffromsql) + appendPQExpBuffer(defqry, ", "); + + if (tosqlFuncInfo) + { + char *fsig = format_function_signature(fout, tosqlFuncInfo, true); + + /* + * Always qualify the function name, in case it is not in + * pg_catalog schema (format_function_signature won't qualify + * it). + */ + appendPQExpBuffer(defqry, "TO SQL WITH FUNCTION %s.%s", + fmtId(tosqlFuncInfo->dobj.namespace->dobj.name), fsig); + free(fsig); + } + else + write_msg(NULL, "WARNING: bogus value in pg_transform.trftosql field\n"); + } + + appendPQExpBuffer(defqry, ");\n"); + + appendPQExpBuffer(labelq, "TRANSFORM FOR %s LANGUAGE %s", + getFormattedTypeName(fout, transform->trftype, zeroAsNone), + lanname); + + if (binary_upgrade) + binary_upgrade_extension_member(defqry, &transform->dobj, labelq->data); + + ArchiveEntry(fout, transform->dobj.catId, transform->dobj.dumpId, + labelq->data, + "pg_catalog", NULL, "", + false, "TRANSFORM", SECTION_PRE_DATA, + defqry->data, delqry->data, NULL, + transform->dobj.dependencies, transform->dobj.nDeps, + NULL, NULL); + + /* Dump Transform Comments */ + dumpComment(fout, labelq->data, + NULL, "", + transform->dobj.catId, 0, transform->dobj.dumpId); + + free(lanname); + destroyPQExpBuffer(defqry); + destroyPQExpBuffer(delqry); + destroyPQExpBuffer(labelq); +} + + +/* * dumpOpr * write out a single operator definition */ @@ -14895,6 +15124,7 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, case DO_TSCONFIG: case DO_FDW: case DO_FOREIGN_SERVER: + case DO_TRANSFORM: case DO_BLOB: /* Pre-data objects: must come before the pre-data boundary */ addObjectDependency(preDataBound, dobj->dumpId); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 5582538..5add50e 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -106,6 +106,7 @@ typedef enum DO_FDW, DO_FOREIGN_SERVER, DO_DEFAULT_ACL, + DO_TRANSFORM, DO_BLOB, DO_BLOB_DATA, DO_PRE_DATA_BOUNDARY, @@ -406,6 +407,15 @@ typedef struct _castInfo char castmethod; } CastInfo; +typedef struct _transformInfo +{ + DumpableObject dobj; + Oid trftype; + Oid trflang; + Oid trffromsql; + Oid trftosql; +} TransformInfo; + /* InhInfo isn't a DumpableObject, just temporary state */ typedef struct _inhInfo { @@ -558,6 +568,7 @@ extern RuleInfo *getRules(Archive *fout, int *numRules); extern void getTriggers(Archive *fout, TableInfo tblinfo[], int numTables); extern ProcLangInfo *getProcLangs(Archive *fout, int *numProcLangs); extern CastInfo *getCasts(Archive *fout, int *numCasts); +extern TransformInfo *getTransforms(Archive *fout, int *numTransforms); extern void getTableAttrs(Archive *fout, TableInfo *tbinfo, int numTables); extern bool shouldPrintColumn(TableInfo *tbinfo, int colno); extern TSParserInfo *getTSParsers(Archive *fout, int *numTSParsers); diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index 141e713..9d0a6d1 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -26,8 +26,8 @@ * by OID. (This is a relatively crude hack to provide semi-reasonable * behavior for old databases without full dependency info.) Note: collations, * extensions, text search, foreign-data, materialized view, event trigger, - * and default ACL objects can't really happen here, so the rather bogus - * priorities for them don't matter. + * transforms, and default ACL objects can't really happen here, so the rather + * bogus priorities for them don't matter. * * NOTE: object-type priorities must match the section assignments made in * pg_dump.c; that is, PRE_DATA objects must sort before DO_PRE_DATA_BOUNDARY, @@ -65,6 +65,7 @@ 4, /* DO_FDW */ 4, /* DO_FOREIGN_SERVER */ 19, /* DO_DEFAULT_ACL */ + 4, /* DO_TRANSFORM */ 9, /* DO_BLOB */ 12, /* DO_BLOB_DATA */ 10, /* DO_PRE_DATA_BOUNDARY */ @@ -113,6 +114,7 @@ 16, /* DO_FDW */ 17, /* DO_FOREIGN_SERVER */ 31, /* DO_DEFAULT_ACL */ + 3, /* DO_TRANSFORM */ 21, /* DO_BLOB */ 24, /* DO_BLOB_DATA */ 22, /* DO_PRE_DATA_BOUNDARY */ @@ -1287,6 +1289,13 @@ static void describeDumpableObject(DumpableObject *obj, ((CastInfo *) obj)->casttarget, obj->dumpId, obj->catId.oid); return; + case DO_TRANSFORM: + snprintf(buf, bufsize, + "TRANSFORM %u lang %u (ID %d OID %u)", + ((TransformInfo *) obj)->trftype, + ((TransformInfo *) obj)->trflang, + obj->dumpId, obj->catId.oid); + return; case DO_TABLE_DATA: snprintf(buf, bufsize, "TABLE DATA %s (ID %d OID %u)", diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index d46fe9e..283d345 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201306121 +#define CATALOG_VERSION_NO 201306132 #endif diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index 3aefbb5e..404fda4 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -147,6 +147,7 @@ typedef enum ObjectClass OCLASS_DEFACL, /* pg_default_acl */ OCLASS_EXTENSION, /* pg_extension */ OCLASS_EVENT_TRIGGER, /* pg_event_trigger */ + OCLASS_TRANSFORM, /* pg_transform */ MAX_OCLASS /* MUST BE LAST */ } ObjectClass; diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index 19268fb..86c7dba 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -226,6 +226,11 @@ DECLARE_UNIQUE_INDEX(pg_tablespace_oid_index, 2697, on pg_tablespace using btree DECLARE_UNIQUE_INDEX(pg_tablespace_spcname_index, 2698, on pg_tablespace using btree(spcname name_ops)); #define TablespaceNameIndexId 2698 +DECLARE_UNIQUE_INDEX(pg_transform_oid_index, 3780, on pg_transform using btree(oid oid_ops)); +#define TransformOidIndexId 3780 +DECLARE_UNIQUE_INDEX(pg_transform_type_lang_index, 3781, on pg_transform using btree(trftype oid_ops, trflang oid_ops)); +#define TransformTypeLangIndexId 3781 + /* This following index is not used for a cache and is not unique */ DECLARE_INDEX(pg_trigger_tgconstraint_index, 2699, on pg_trigger using btree(tgconstraint oid_ops)); #define TriggerConstraintIndexId 2699 diff --git a/src/include/catalog/pg_transform.h b/src/include/catalog/pg_transform.h new file mode 100644 index 0000000..80b191e --- /dev/null +++ b/src/include/catalog/pg_transform.h @@ -0,0 +1,47 @@ +/*------------------------------------------------------------------------- + * + * pg_transform.h + * + * Copyright (c) 2012, PostgreSQL Global Development Group + * + * src/include/catalog/pg_transform.h + * + * NOTES + * the genbki.pl script reads this file and generates .bki + * information from the DATA() statements. + * + *------------------------------------------------------------------------- + */ +#ifndef PG_TRANSFORM_H +#define PG_TRANSFORM_H + +#include "catalog/genbki.h" + +/* ---------------- + * pg_transform definition. cpp turns this into + * typedef struct FormData_pg_transform + * ---------------- + */ +#define TransformRelationId 3779 + +CATALOG(pg_transform,3779) +{ + Oid trftype; + Oid trflang; + regproc trffromsql; + regproc trftosql; +} FormData_pg_transform; + +typedef FormData_pg_transform *Form_pg_transform; + +/* ---------------- + * compiler constants for pg_transform + * ---------------- + */ +#define Natts_pg_transform 4 +#define Anum_pg_transform_trftype 1 +#define Anum_pg_transform_trflang 2 +#define Anum_pg_transform_trffromsql 3 +#define Anum_pg_transform_trftosql 4 + +#endif /* PG_TRANSFORM_H */ diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index fa9f41f..deac301 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -49,10 +49,13 @@ extern void SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType); extern Oid AlterFunction(AlterFunctionStmt *stmt); extern Oid CreateCast(CreateCastStmt *stmt); extern void DropCastById(Oid castOid); +extern Oid CreateTransform(CreateTransformStmt *stmt); +extern void DropTransformById(Oid transformOid); extern void IsThereFunctionInNamespace(const char *proname, int pronargs, oidvector *proargtypes, Oid nspOid); extern void ExecuteDoStmt(DoStmt *stmt); extern Oid get_cast_oid(Oid sourcetypeid, Oid targettypeid, bool missing_ok); +extern Oid get_transform_oid(Oid typeid, Oid langid, bool missing_ok); /* commands/operatorcmds.c */ extern Oid DefineOperator(List *names, List *parameters); diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h index 0d5c007..1c082e9 100644 --- a/src/include/nodes/nodes.h +++ b/src/include/nodes/nodes.h @@ -362,6 +362,7 @@ typedef enum NodeTag T_CreateEventTrigStmt, T_AlterEventTrigStmt, T_RefreshMatViewStmt, + T_CreateTransformStmt, /* * TAGS FOR PARSE TREE NODES (parsenodes.h) diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 6723647..f028068 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1145,6 +1145,7 @@ typedef enum ObjectType OBJECT_SEQUENCE, OBJECT_TABLE, OBJECT_TABLESPACE, + OBJECT_TRANSFORM, OBJECT_TRIGGER, OBJECT_TSCONFIGURATION, OBJECT_TSDICTIONARY, @@ -2569,6 +2570,20 @@ typedef struct CreateCastStmt } CreateCastStmt; /* ---------------------- + * CREATE TRANSFORM Statement + * ---------------------- + */ +typedef struct CreateTransformStmt +{ + NodeTag type; + bool replace; + TypeName *type_name; + char *lang; + FuncWithArgs *fromsql; + FuncWithArgs *tosql; +} CreateTransformStmt; + +/* ---------------------- * PREPARE Statement * ---------------------- */ diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index 68a13b7..88e9ece 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -345,6 +345,7 @@ PG_KEYWORD("simple", SIMPLE, UNRESERVED_KEYWORD) PG_KEYWORD("smallint", SMALLINT, COL_NAME_KEYWORD) PG_KEYWORD("snapshot", SNAPSHOT, UNRESERVED_KEYWORD) PG_KEYWORD("some", SOME, RESERVED_KEYWORD) +PG_KEYWORD("sql", SQL_P, UNRESERVED_KEYWORD) PG_KEYWORD("stable", STABLE, UNRESERVED_KEYWORD) PG_KEYWORD("standalone", STANDALONE_P, UNRESERVED_KEYWORD) PG_KEYWORD("start", START, UNRESERVED_KEYWORD) @@ -372,6 +373,7 @@ PG_KEYWORD("timestamp", TIMESTAMP, COL_NAME_KEYWORD) PG_KEYWORD("to", TO, RESERVED_KEYWORD) PG_KEYWORD("trailing", TRAILING, RESERVED_KEYWORD) PG_KEYWORD("transaction", TRANSACTION, UNRESERVED_KEYWORD) +PG_KEYWORD("transform", TRANSFORM, UNRESERVED_KEYWORD) PG_KEYWORD("treat", TREAT, COL_NAME_KEYWORD) PG_KEYWORD("trigger", TRIGGER, UNRESERVED_KEYWORD) PG_KEYWORD("trim", TRIM, COL_NAME_KEYWORD) diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index 49f459a..9ff411f 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -72,6 +72,7 @@ extern void get_atttypetypmodcoll(Oid relid, AttrNumber attnum, Oid *typid, int32 *typmod, Oid *collid); extern char *get_collation_name(Oid colloid); extern char *get_constraint_name(Oid conoid); +extern char *get_language_name(Oid langoid, bool missing_ok); extern Oid get_opclass_family(Oid opclass); extern Oid get_opclass_input_type(Oid opclass); extern RegProcedure get_opcode(Oid opno); @@ -102,6 +103,9 @@ extern Oid get_rel_namespace(Oid relid); extern Oid get_rel_type_id(Oid relid); extern char get_rel_relkind(Oid relid); extern Oid get_rel_tablespace(Oid relid); +extern Oid get_transform(Oid typid, Oid langid); +extern Oid get_transform_fromsql(Oid typid, Oid langid); +extern Oid get_transform_tosql(Oid typid, Oid langid); extern bool get_typisdefined(Oid typid); extern int16 get_typlen(Oid typid); extern bool get_typbyval(Oid typid); diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h index d1d8abe..ee482f5 100644 --- a/src/include/utils/syscache.h +++ b/src/include/utils/syscache.h @@ -80,6 +80,8 @@ enum SysCacheIdentifier RULERELNAME, STATRELATTINH, TABLESPACEOID, + TRFOID, + TRFTYPELANG, TSCONFIGMAP, TSCONFIGNAMENSP, TSCONFIGOID, diff --git a/src/interfaces/ecpg/preproc/ecpg.tokens b/src/interfaces/ecpg/preproc/ecpg.tokens index b55138a..68ba925 100644 --- a/src/interfaces/ecpg/preproc/ecpg.tokens +++ b/src/interfaces/ecpg/preproc/ecpg.tokens @@ -12,7 +12,7 @@ SQL_LONG SQL_NULLABLE SQL_OCTET_LENGTH SQL_OPEN SQL_OUTPUT SQL_REFERENCE SQL_RETURNED_LENGTH SQL_RETURNED_OCTET_LENGTH SQL_SCALE - SQL_SECTION SQL_SHORT SQL_SIGNED SQL_SQL SQL_SQLERROR + SQL_SECTION SQL_SHORT SQL_SIGNED SQL_SQLERROR SQL_SQLPRINT SQL_SQLWARNING SQL_START SQL_STOP SQL_STRUCT SQL_UNSIGNED SQL_VAR SQL_WHENEVER diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer b/src/interfaces/ecpg/preproc/ecpg.trailer index 8258ce2..a6f3af5 100644 --- a/src/interfaces/ecpg/preproc/ecpg.trailer +++ b/src/interfaces/ecpg/preproc/ecpg.trailer @@ -1000,7 +1000,7 @@ ecpg_using: USING using_list { $$ = EMPTY; } | using_descriptor { $$ = $1; } ; -using_descriptor: USING SQL_SQL SQL_DESCRIPTOR quoted_ident_stringvar +using_descriptor: USING SQL_P SQL_DESCRIPTOR quoted_ident_stringvar { add_variable_to_head(&argsinsert, descriptor_variable($4,0), &no_indicator); $$ = EMPTY; @@ -1012,7 +1012,7 @@ using_descriptor: USING SQL_SQL SQL_DESCRIPTOR quoted_ident_stringvar } ; -into_descriptor: INTO SQL_SQL SQL_DESCRIPTOR quoted_ident_stringvar +into_descriptor: INTO SQL_P SQL_DESCRIPTOR quoted_ident_stringvar { add_variable_to_head(&argsresult, descriptor_variable($4,1), &no_indicator); $$ = EMPTY; @@ -1489,7 +1489,6 @@ ECPGKeywords_vanames: SQL_BREAK { $$ = mm_strdup("break"); } | SQL_RETURNED_OCTET_LENGTH { $$ = mm_strdup("returned_octet_length"); } | SQL_SCALE { $$ = mm_strdup("scale"); } | SQL_SECTION { $$ = mm_strdup("section"); } - | SQL_SQL { $$ = mm_strdup("sql"); } | SQL_SQLERROR { $$ = mm_strdup("sqlerror"); } | SQL_SQLPRINT { $$ = mm_strdup("sqlprint"); } | SQL_SQLWARNING { $$ = mm_strdup("sqlwarning"); } diff --git a/src/interfaces/ecpg/preproc/ecpg_keywords.c b/src/interfaces/ecpg/preproc/ecpg_keywords.c index fb54d7b..6c819fd 100644 --- a/src/interfaces/ecpg/preproc/ecpg_keywords.c +++ b/src/interfaces/ecpg/preproc/ecpg_keywords.c @@ -67,8 +67,6 @@ {"section", SQL_SECTION, 0}, {"short", SQL_SHORT, 0}, {"signed", SQL_SIGNED, 0}, - {"sql", SQL_SQL, 0}, /* strange thing, used for into sql descriptor - * MYDESC; */ {"sqlerror", SQL_SQLERROR, 0}, {"sqlprint", SQL_SQLPRINT, 0}, {"sqlwarning", SQL_SQLWARNING, 0}, diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index e0e31ec..49a4ed8 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -79,15 +79,17 @@ Util.c: Util.xs plperl_helpers.h install: all install-lib install-data installdirs: installdirs-lib - $(MKDIR_P) '$(DESTDIR)$(datadir)/extension' + $(MKDIR_P) '$(DESTDIR)$(datadir)/extension' '$(DESTDIR)$(includedir_server)' uninstall: uninstall-lib uninstall-data install-data: installdirs $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) '$(DESTDIR)$(datadir)/extension/' + $(INSTALL_DATA) $(srcdir)/plperl.h $(srcdir)/ppport.h '$(DESTDIR)$(includedir_server)' uninstall-data: rm -f $(addprefix '$(DESTDIR)$(datadir)/extension'/, $(notdir $(DATA))) + rm -f $(addprefix '$(DESTDIR)$(includedir_server)'/, plperl.h ppport.h) .PHONY: install-data uninstall-data diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index de8cb0e..851a41f 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -110,6 +110,7 @@ SV *reference; /* CODE reference for Perl sub */ plperl_interp_desc *interp; /* interpreter it's created in */ bool fn_readonly; /* is function readonly (not volatile)? */ + Oid lang_oid; bool lanpltrusted; /* is it plperl, rather than plperlu? */ bool fn_retistuple; /* true, if function returns tuple */ bool fn_retisset; /* true, if function returns set */ @@ -210,6 +211,7 @@ bool *nulls; int *nelems; FmgrInfo proc; + FmgrInfo transform_proc; } plperl_array_info; /********************************************************************** @@ -1260,6 +1262,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 +1286,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 */ @@ -1395,6 +1400,7 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, typdelim; Oid typioparam; Oid typoutputfunc; + Oid transform_funcid; int i, nitems, *dims; @@ -1402,14 +1408,17 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, SV *av; HV *hv; - info = palloc(sizeof(plperl_array_info)); + info = palloc0(sizeof(plperl_array_info)); /* get element type information, including output conversion function */ get_type_io_data(elementtype, IOFunc_output, &typlen, &typbyval, &typalign, &typdelim, &typioparam, &typoutputfunc); - perm_fmgr_info(typoutputfunc, &info->proc); + if ((transform_funcid = get_transform_fromsql(elementtype, current_call_data->prodesc->lang_oid))) + perm_fmgr_info(transform_funcid, &info->transform_proc); + else + perm_fmgr_info(typoutputfunc, &info->proc); info->elem_is_rowtype = type_is_rowtype(elementtype); @@ -1490,8 +1499,10 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, { Datum itemvalue = info->elements[i]; - /* Handle composite type elements */ - if (info->elem_is_rowtype) + if (info->transform_proc.fn_oid) + av_push(result, (SV *) DatumGetPointer(FunctionCall1(&info->transform_proc, itemvalue))); + else if (info->elem_is_rowtype) + /* Handle composite type elements */ av_push(result, plperl_hash_from_datum(itemvalue)); else { @@ -1778,6 +1789,7 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, desc.proname = "inline_code_block"; desc.fn_readonly = false; + desc.lang_oid = codeblock->langOid; desc.lanpltrusted = codeblock->langIsTrusted; desc.fn_retistuple = false; @@ -2035,6 +2047,8 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, SV *retval; int i; int count; + Oid *argtypes = NULL; + int nargs = 0; ENTER; SAVETMPS; @@ -2042,6 +2056,9 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, PUSHMARK(SP); EXTEND(sp, desc->nargs); + if (fcinfo->flinfo->fn_oid) + get_func_signature(fcinfo->flinfo->fn_oid, &argtypes, &nargs); + for (i = 0; i < desc->nargs; i++) { if (fcinfo->argnull[i]) @@ -2055,9 +2072,12 @@ 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 ((funcid = get_transform_fromsql(argtypes[i], current_call_data->prodesc->lang_oid))) + sv = (SV *) DatumGetPointer(OidFunctionCall1(funcid, fcinfo->arg[i])); else { char *tmp; @@ -2536,6 +2556,7 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, procStruct->prolang); } langStruct = (Form_pg_language) GETSTRUCT(langTup); + prodesc->lang_oid = HeapTupleGetOid(langTup); prodesc->lanpltrusted = langStruct->lanpltrusted; ReleaseSysCache(langTup); @@ -2768,9 +2789,12 @@ static SV *plperl_call_perl_func(plperl_proc_desc *desc, else { SV *sv; + Oid funcid; if (OidIsValid(get_base_element_type(tupdesc->attrs[i]->atttypid))) sv = plperl_ref_from_pg_array(attr, tupdesc->attrs[i]->atttypid); + else if ((funcid = get_transform_fromsql(tupdesc->attrs[i]->atttypid, current_call_data->prodesc->lang_oid))) + sv = (SV *) DatumGetPointer(OidFunctionCall1(funcid, attr)); else { char *outputstr; diff --git a/src/pl/plperl/plperl_helpers.h b/src/pl/plperl/plperl_helpers.h index 3e8aa7c..53ff66a 100644 --- a/src/pl/plperl/plperl_helpers.h +++ b/src/pl/plperl/plperl_helpers.h @@ -1,6 +1,8 @@ #ifndef PL_PERL_HELPERS_H #define PL_PERL_HELPERS_H +#include "mb/pg_wchar.h" + /* * convert from utf8 to database encoding * diff --git a/src/pl/plpython/Makefile b/src/pl/plpython/Makefile index 3fe8e4a..d595c5c 100644 --- a/src/pl/plpython/Makefile +++ b/src/pl/plpython/Makefile @@ -115,54 +115,22 @@ all: all-lib install: all install-lib install-data installdirs: installdirs-lib - $(MKDIR_P) '$(DESTDIR)$(datadir)/extension' + $(MKDIR_P) '$(DESTDIR)$(datadir)/extension' '$(DESTDIR)$(includedir_server)' uninstall: uninstall-lib uninstall-data install-data: installdirs $(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) '$(DESTDIR)$(datadir)/extension/' + $(INSTALL_DATA) $(srcdir)/plpython.h $(srcdir)/plpy_util.h '$(DESTDIR)$(includedir_server)' uninstall-data: rm -f $(addprefix '$(DESTDIR)$(datadir)/extension'/, $(notdir $(DATA))) + rm -f $(addprefix '$(DESTDIR)$(includedir_server)'/, plpython.h plpy_util.h) .PHONY: install-data uninstall-data -ifeq ($(python_majorversion),3) -# Adjust regression tests for Python 3 compatibility -# -# Mention those regression test files that need to be mangled in the -# variable REGRESS_PLPYTHON3_MANGLE. They will be copied to a -# subdirectory python3/ and have their Python syntax and other bits -# adjusted to work with Python 3. - -# Note that the order of the tests needs to be preserved in this -# expression. -REGRESS := $(foreach test,$(REGRESS),$(if $(filter $(test),$(REGRESS_PLPYTHON3_MANGLE)),python3/$(test),$(test))) - -.PHONY: pgregress-python3-mangle -pgregress-python3-mangle: - $(MKDIR_P) sql/python3 expected/python3 results/python3 - for file in $(patsubst %,$(srcdir)/sql/%.sql,$(REGRESS_PLPYTHON3_MANGLE)) $(patsubst %,$(srcdir)/expected/%*.out,$(REGRESS_PLPYTHON3_MANGLE));do \ - sed -e 's/except \([[:alpha:]][[:alpha:].]*\), *\([[:alpha:]][[:alpha:]]*\):/except \1 as \2:/g' \ - -e "s/<type 'exceptions\.\([[:alpha:]]*\)'>/<class '\1'>/g" \ - -e "s/<type 'long'>/<class 'int'>/g" \ - -e "s/\([0-9][0-9]*\)L/\1/g" \ - -e 's/\([ [{]\)u"/\1"/g' \ - -e "s/\([ [{]\)u'/\1'/g" \ - -e "s/def next/def __next__/g" \ - -e "s/LANGUAGE plpythonu/LANGUAGE plpython3u/g" \ - -e "s/LANGUAGE plpython2u/LANGUAGE plpython3u/g" \ - -e "s/EXTENSION plpythonu/EXTENSION plpython3u/g" \ - -e "s/EXTENSION plpython2u/EXTENSION plpython3u/g" \ - $$file >`echo $$file | sed 's,^.*/\([^/][^/]*/\)\([^/][^/]*\)$$,\1python3/\2,'` || exit; \ - done - -check installcheck: pgregress-python3-mangle - -pg_regress_clean_files += sql/python3/ expected/python3/ results/python3/ - -endif # Python 3 +include $(srcdir)/regress-python3-mangle.mk check: all submake diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c index 0dad843..10cb623 100644 --- a/src/pl/plpython/plpy_main.c +++ b/src/pl/plpython/plpy_main.c @@ -283,6 +283,7 @@ MemSet(&proc, 0, sizeof(PLyProcedure)); proc.pyname = PLy_strdup("__plpython_inline_block"); + proc.langid = codeblock->langOid; proc.result.out.d.typoid = VOIDOID; /* diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index 5007e77..f8ee305 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -166,6 +166,7 @@ for (i = 0; i < FUNC_MAX_ARGS; i++) PLy_typeinfo_init(&proc->args[i]); proc->nargs = 0; + proc->langid = procStruct->prolang; proc->code = proc->statics = NULL; proc->globals = NULL; proc->is_setof = procStruct->proretset; @@ -220,7 +221,7 @@ else { /* do the real work */ - PLy_output_datum_func(&proc->result, rvTypeTup); + PLy_output_datum_func(&proc->result, rvTypeTup, proc->langid); } ReleaseSysCache(rvTypeTup); @@ -294,7 +295,8 @@ default: PLy_input_datum_func(&(proc->args[pos]), types[i], - argTypeTup); + argTypeTup, + proc->langid); break; } diff --git a/src/pl/plpython/plpy_procedure.h b/src/pl/plpython/plpy_procedure.h index f1c8510..9776e7d 100644 --- a/src/pl/plpython/plpy_procedure.h +++ b/src/pl/plpython/plpy_procedure.h @@ -27,6 +27,7 @@ typedef struct PLyProcedure char **argnames; /* Argument names */ PLyTypeInfo args[FUNC_MAX_ARGS]; int nargs; + Oid langid; /* OID of plpython pg_language entry */ PyObject *code; /* compiled procedure code */ PyObject *statics; /* data saved across calls, local scope */ PyObject *globals; /* data saved across calls, global scope */ diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c index c9182eb..2fa255d 100644 --- a/src/pl/plpython/plpy_spi.c +++ b/src/pl/plpython/plpy_spi.c @@ -76,6 +76,7 @@ PG_TRY(); { int i; + PLyExecutionContext *exec_ctx = PLy_current_execution_context(); /* * the other loop might throw an exception, if PLyTypeInfo member @@ -131,7 +132,7 @@ plan->types[i] = typeId; typeStruct = (Form_pg_type) GETSTRUCT(typeTup); if (typeStruct->typtype != TYPTYPE_COMPOSITE) - PLy_output_datum_func(&plan->args[i], typeTup); + PLy_output_datum_func(&plan->args[i], typeTup, exec_ctx->curr_proc->langid); else ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c index 8f2367d..f670ebe 100644 --- a/src/pl/plpython/plpy_typeio.c +++ b/src/pl/plpython/plpy_typeio.c @@ -28,8 +28,8 @@ /* I/O function caching */ -static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup); -static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup); +static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup, Oid langid); +static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup, Oid langid); /* conversion from Datums to Python objects */ static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d); @@ -42,6 +42,7 @@ static PyObject *PLyLong_FromOid(PLyDatumToOb *arg, Datum d); static PyObject *PLyBytes_FromBytea(PLyDatumToOb *arg, Datum d); static PyObject *PLyString_FromDatum(PLyDatumToOb *arg, Datum d); +static PyObject *PLyObject_FromTransform(PLyDatumToOb *arg, Datum d); static PyObject *PLyList_FromArray(PLyDatumToOb *arg, Datum d); /* conversion from Python objects to Datums */ @@ -49,6 +50,7 @@ static Datum PLyObject_ToBytea(PLyObToDatum *arg, int32 typmod, PyObject *plrv); static Datum PLyObject_ToComposite(PLyObToDatum *arg, int32 typmod, PyObject *plrv); static Datum PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv); +static Datum PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv); static Datum PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv); /* conversion from Python objects to composite Datums (used by triggers and SRFs) */ @@ -101,27 +103,28 @@ * PostgreSQL, and vice versa. */ void -PLy_input_datum_func(PLyTypeInfo *arg, Oid typeOid, HeapTuple typeTup) +PLy_input_datum_func(PLyTypeInfo *arg, Oid typeOid, HeapTuple typeTup, Oid langid) { if (arg->is_rowtype > 0) elog(ERROR, "PLyTypeInfo struct is initialized for Tuple"); arg->is_rowtype = 0; - PLy_input_datum_func2(&(arg->in.d), typeOid, typeTup); + PLy_input_datum_func2(&(arg->in.d), typeOid, typeTup, langid); } void -PLy_output_datum_func(PLyTypeInfo *arg, HeapTuple typeTup) +PLy_output_datum_func(PLyTypeInfo *arg, HeapTuple typeTup, Oid langid) { if (arg->is_rowtype > 0) elog(ERROR, "PLyTypeInfo struct is initialized for a Tuple"); arg->is_rowtype = 0; - PLy_output_datum_func2(&(arg->out.d), typeTup); + PLy_output_datum_func2(&(arg->out.d), typeTup, langid); } void PLy_input_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc) { int i; + PLyExecutionContext *exec_ctx = PLy_current_execution_context(); if (arg->is_rowtype == 0) elog(ERROR, "PLyTypeInfo struct is initialized for a Datum"); @@ -180,7 +183,8 @@ PLy_input_datum_func2(&(arg->in.r.atts[i]), desc->attrs[i]->atttypid, - typeTup); + typeTup, + exec_ctx->curr_proc->langid); ReleaseSysCache(typeTup); } @@ -190,6 +194,7 @@ PLy_output_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc) { int i; + PLyExecutionContext *exec_ctx = PLy_current_execution_context(); if (arg->is_rowtype == 0) elog(ERROR, "PLyTypeInfo struct is initialized for a Datum"); @@ -242,7 +247,7 @@ elog(ERROR, "cache lookup failed for type %u", desc->attrs[i]->atttypid); - PLy_output_datum_func2(&(arg->out.r.atts[i]), typeTup); + PLy_output_datum_func2(&(arg->out.r.atts[i]), typeTup, exec_ctx->curr_proc->langid); ReleaseSysCache(typeTup); } @@ -361,10 +366,12 @@ } static void -PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup) +PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup, Oid langid) { Form_pg_type typeStruct = (Form_pg_type) GETSTRUCT(typeTup); Oid element_type; + Oid base_type; + Oid funcid; perm_fmgr_info(typeStruct->typinput, &arg->typfunc); arg->typoid = HeapTupleGetOid(typeTup); @@ -373,12 +380,24 @@ arg->typbyval = typeStruct->typbyval; element_type = get_element_type(arg->typoid); + base_type = getBaseType(element_type ? element_type : arg->typoid); /* * Select a conversion function to convert Python objects to PostgreSQL - * datums. Most data types can go through the generic function. + * datums. */ - switch (getBaseType(element_type ? element_type : arg->typoid)) + + if ((funcid = get_transform_tosql(base_type, langid))) + { + arg->func = PLyObject_ToTransform; + perm_fmgr_info(funcid, &arg->typtransform); + } + else if (typeStruct->typtype == TYPTYPE_COMPOSITE) + { + arg->func = PLyObject_ToComposite; + } + else + switch (base_type) { case BOOLOID: arg->func = PLyObject_ToBool; @@ -391,12 +410,6 @@ break; } - /* Composite types need their own input routine, though */ - if (typeStruct->typtype == TYPTYPE_COMPOSITE) - { - arg->func = PLyObject_ToComposite; - } - if (element_type) { char dummy_delim; @@ -411,6 +424,7 @@ arg->elm = PLy_malloc0(sizeof(*arg->elm)); arg->elm->func = arg->func; + arg->elm->typtransform = arg->typtransform; arg->func = PLySequence_ToArray; arg->elm->typoid = element_type; @@ -423,10 +437,12 @@ } static void -PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup) +PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup, Oid langid) { Form_pg_type typeStruct = (Form_pg_type) GETSTRUCT(typeTup); - Oid element_type = get_element_type(typeOid); + Oid element_type; + Oid base_type; + Oid funcid; /* Get the type's conversion information */ perm_fmgr_info(typeStruct->typoutput, &arg->typfunc); @@ -438,7 +454,17 @@ arg->typalign = typeStruct->typalign; /* Determine which kind of Python object we will convert to */ - switch (getBaseType(element_type ? element_type : typeOid)) + + element_type = get_element_type(typeOid); + base_type = getBaseType(element_type ? element_type : typeOid); + + if ((funcid = get_transform_fromsql(base_type, langid))) + { + arg->func = PLyObject_FromTransform; + perm_fmgr_info(funcid, &arg->typtransform); + } + else + switch (base_type) { case BOOLOID: arg->func = PLyBool_FromBool; @@ -479,6 +505,7 @@ arg->elm = PLy_malloc0(sizeof(*arg->elm)); arg->elm->func = arg->func; + arg->elm->typtransform = arg->typtransform; arg->func = PLyList_FromArray; arg->elm->typoid = element_type; arg->elm->typmod = -1; @@ -569,14 +596,22 @@ static PyObject * PLyString_FromDatum(PLyDatumToOb *arg, Datum d) { - char *x = OutputFunctionCall(&arg->typfunc, d); - PyObject *r = PyString_FromString(x); + char *x; + PyObject *r; + x = OutputFunctionCall(&arg->typfunc, d); + r = PyString_FromString(x); pfree(x); return r; } static PyObject * +PLyObject_FromTransform(PLyDatumToOb *arg, Datum d) +{ + return (PyObject *) DatumGetPointer(FunctionCall1(&arg->typtransform, d)); +} + +static PyObject * PLyList_FromArray(PLyDatumToOb *arg, Datum d) { ArrayType *array = DatumGetArrayTypeP(d); @@ -725,16 +760,15 @@ /* - * Generic conversion function: Convert PyObject to cstring and - * cstring into PostgreSQL type. + * Convert Python object to C string in server encoding. */ -static Datum -PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv) +char * +PLyObject_AsString(PyObject *plrv) { - PyObject *volatile plrv_bo = NULL; - Datum rv; - - Assert(plrv != Py_None); + PyObject *plrv_bo; + char *plrv_sc; + size_t plen; + size_t slen; if (PyUnicode_Check(plrv)) plrv_bo = PLyUnicode_Bytes(plrv); @@ -752,36 +786,47 @@ if (!plrv_bo) PLy_elog(ERROR, "could not create string representation of Python object"); - PG_TRY(); - { - char *plrv_sc = PyBytes_AsString(plrv_bo); - size_t plen = PyBytes_Size(plrv_bo); - size_t slen = strlen(plrv_sc); - - if (slen < plen) - ereport(ERROR, - (errcode(ERRCODE_DATATYPE_MISMATCH), - errmsg("could not convert Python object into cstring: Python string representation appears to containnull bytes"))); - else if (slen > plen) - elog(ERROR, "could not convert Python object into cstring: Python string longer than reported length"); - pg_verifymbstr(plrv_sc, slen, false); - rv = InputFunctionCall(&arg->typfunc, - plrv_sc, - arg->typioparam, - typmod); - } - PG_CATCH(); - { - Py_XDECREF(plrv_bo); - PG_RE_THROW(); - } - PG_END_TRY(); + plrv_sc = pstrdup(PyBytes_AsString(plrv_bo)); + plen = PyBytes_Size(plrv_bo); + slen = strlen(plrv_sc); Py_XDECREF(plrv_bo); - return rv; + if (slen < plen) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("could not convert Python object into cstring: Python string representation appears to contain nullbytes"))); + else if (slen > plen) + elog(ERROR, "could not convert Python object into cstring: Python string longer than reported length"); + pg_verifymbstr(plrv_sc, slen, false); + + return plrv_sc; } + +/* + * Generic conversion function: Convert PyObject to cstring and + * cstring into PostgreSQL type. + */ +static Datum +PLyObject_ToDatum(PLyObToDatum *arg, int32 typmod, PyObject *plrv) +{ + Assert(plrv != Py_None); + + return InputFunctionCall(&arg->typfunc, + PLyObject_AsString(plrv), + arg->typioparam, + typmod); +} + + +static Datum +PLyObject_ToTransform(PLyObToDatum *arg, int32 typmod, PyObject *plrv) +{ + return FunctionCall1(&arg->typtransform, PointerGetDatum(plrv)); +} + + static Datum PLySequence_ToArray(PLyObToDatum *arg, int32 typmod, PyObject *plrv) { @@ -831,12 +876,13 @@ PLyString_ToComposite(PLyTypeInfo *info, TupleDesc desc, PyObject *string) { HeapTuple typeTup; + PLyExecutionContext *exec_ctx = PLy_current_execution_context(); typeTup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(desc->tdtypeid)); if (!HeapTupleIsValid(typeTup)) elog(ERROR, "cache lookup failed for type %u", desc->tdtypeid); - PLy_output_datum_func2(&info->out.d, typeTup); + PLy_output_datum_func2(&info->out.d, typeTup, exec_ctx->curr_proc->langid); ReleaseSysCache(typeTup); ReleaseTupleDesc(desc); diff --git a/src/pl/plpython/plpy_typeio.h b/src/pl/plpython/plpy_typeio.h index 82e472a..b869aaa 100644 --- a/src/pl/plpython/plpy_typeio.h +++ b/src/pl/plpython/plpy_typeio.h @@ -17,6 +17,7 @@ typedef struct PLyDatumToOb { PLyDatumToObFunc func; FmgrInfo typfunc; /* The type's output function */ + FmgrInfo typtransform; /* from-SQL transform */ Oid typoid; /* The OID of the type */ int32 typmod; /* The typmod of the type */ Oid typioparam; @@ -48,6 +49,7 @@ typedef struct PLyObToDatum { PLyObToDatumFunc func; FmgrInfo typfunc; /* The type's input function */ + FmgrInfo typtransform; /* to-SQL transform */ Oid typoid; /* The OID of the type */ int32 typmod; /* The typmod of the type */ Oid typioparam; @@ -91,8 +93,8 @@ typedef struct PLyTypeInfo extern void PLy_typeinfo_init(PLyTypeInfo *arg); extern void PLy_typeinfo_dealloc(PLyTypeInfo *arg); -extern void PLy_input_datum_func(PLyTypeInfo *arg, Oid typeOid, HeapTuple typeTup); -extern void PLy_output_datum_func(PLyTypeInfo *arg, HeapTuple typeTup); +extern void PLy_input_datum_func(PLyTypeInfo *arg, Oid typeOid, HeapTuple typeTup, Oid langid); +extern void PLy_output_datum_func(PLyTypeInfo *arg, HeapTuple typeTup, Oid langid); extern void PLy_input_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc); extern void PLy_output_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc); @@ -105,4 +107,7 @@ extern Datum PLyObject_ToCompositeDatum(PLyTypeInfo *info, TupleDesc desc, PyObj /* conversion from heap tuples to Python dictionaries */ extern PyObject *PLyDict_FromTuple(PLyTypeInfo *info, HeapTuple tuple, TupleDesc desc); +/* conversion from Python objects to C strings */ +extern char *PLyObject_AsString(PyObject *plrv); + #endif /* PLPY_TYPEIO_H */ diff --git a/src/pl/plpython/plpy_util.c b/src/pl/plpython/plpy_util.c index 95cbba5..bfa09d8 100644 --- a/src/pl/plpython/plpy_util.c +++ b/src/pl/plpython/plpy_util.c @@ -144,22 +144,33 @@ * unicode object. Reference ownership is passed to the caller. */ PyObject * -PLyUnicode_FromString(const char *s) +PLyUnicode_FromStringAndSize(const char *s, Py_ssize_t size) { char *utf8string; PyObject *o; utf8string = (char *) pg_do_encoding_conversion((unsigned char *) s, - strlen(s), + size, GetDatabaseEncoding(), PG_UTF8); - o = PyUnicode_FromString(utf8string); - - if (utf8string != s) + if (utf8string == s) + { + o = PyUnicode_FromStringAndSize(s, size); + } + else + { + o = PyUnicode_FromString(utf8string); pfree(utf8string); + } return o; } +PyObject * +PLyUnicode_FromString(const char *s) +{ + return PLyUnicode_FromStringAndSize(s, strlen(s)); +} + #endif /* PY_MAJOR_VERSION >= 3 */ diff --git a/src/pl/plpython/plpy_util.h b/src/pl/plpython/plpy_util.h index f93e837..4c29f9a 100644 --- a/src/pl/plpython/plpy_util.h +++ b/src/pl/plpython/plpy_util.h @@ -16,6 +16,7 @@ extern char *PLyUnicode_AsString(PyObject *unicode); #if PY_MAJOR_VERSION >= 3 extern PyObject *PLyUnicode_FromString(const char *s); +extern PyObject *PLyUnicode_FromStringAndSize(const char *s, Py_ssize_t size); #endif #endif /* PLPY_UTIL_H */ diff --git a/src/pl/plpython/plpython.h b/src/pl/plpython/plpython.h index 795231c..632b09a 100644 --- a/src/pl/plpython/plpython.h +++ b/src/pl/plpython/plpython.h @@ -91,6 +91,7 @@ typedef int Py_ssize_t; #define PyString_Check(x) 0 #define PyString_AsString(x) PLyUnicode_AsString(x) #define PyString_FromString(x) PLyUnicode_FromString(x) +#define PyString_FromStringAndSize(x, size) PLyUnicode_FromStringAndSize(x, size) #endif /* diff --git a/src/pl/plpython/regress-python3-mangle.mk b/src/pl/plpython/regress-python3-mangle.mk new file mode 100644 index 0000000..d2c7490 --- /dev/null +++ b/src/pl/plpython/regress-python3-mangle.mk @@ -0,0 +1,35 @@ +ifeq ($(python_majorversion),3) +# Adjust regression tests for Python 3 compatibility +# +# Mention those regression test files that need to be mangled in the +# variable REGRESS_PLPYTHON3_MANGLE. They will be copied to a +# subdirectory python3/ and have their Python syntax and other bits +# adjusted to work with Python 3. + +# Note that the order of the tests needs to be preserved in this +# expression. +REGRESS := $(foreach test,$(REGRESS),$(if $(filter $(test),$(REGRESS_PLPYTHON3_MANGLE)),python3/$(test),$(test))) + +.PHONY: pgregress-python3-mangle +pgregress-python3-mangle: + $(MKDIR_P) sql/python3 expected/python3 results/python3 + for file in $(patsubst %,$(srcdir)/sql/%.sql,$(REGRESS_PLPYTHON3_MANGLE)) $(patsubst %,$(srcdir)/expected/%*.out,$(REGRESS_PLPYTHON3_MANGLE));do \ + sed -e 's/except \([[:alpha:]][[:alpha:].]*\), *\([[:alpha:]][[:alpha:]]*\):/except \1 as \2:/g' \ + -e "s/<type 'exceptions\.\([[:alpha:]]*\)'>/<class '\1'>/g" \ + -e "s/<type 'long'>/<class 'int'>/g" \ + -e "s/\([0-9][0-9]*\)L/\1/g" \ + -e 's/\([ [{]\)u"/\1"/g' \ + -e "s/\([ [{]\)u'/\1'/g" \ + -e "s/def next/def __next__/g" \ + -e "s/LANGUAGE plpythonu/LANGUAGE plpython3u/g" \ + -e "s/LANGUAGE plpython2u/LANGUAGE plpython3u/g" \ + -e "s/EXTENSION \([^ ]*_\)*plpythonu/EXTENSION \1plpython3u/g" \ + -e "s/EXTENSION \([^ ]*_\)*plpython2u/EXTENSION \1plpython3u/g" \ + $$file >`echo $$file | sed 's,^.*/\([^/][^/]*/\)\([^/][^/]*\)$$,\1python3/\2,'` || exit; \ + done + +check installcheck: pgregress-python3-mangle + +pg_regress_clean_files += sql/python3/ expected/python3/ results/python3/ + +endif # Python 3 diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out index 432d39a..85fb3f7 100644 --- a/src/test/regress/expected/sanity_check.out +++ b/src/test/regress/expected/sanity_check.out @@ -126,6 +126,7 @@ SELECT relname, relhasindex pg_shseclabel | t pg_statistic | t pg_tablespace | t + pg_transform | t pg_trigger | t pg_ts_config | t pg_ts_config_map | t @@ -166,7 +167,7 @@ SELECT relname, relhasindex timetz_tbl | f tinterval_tbl | f varchar_tbl | f -(155 rows) +(156 rows) -- -- another sanity check: every system catalog that has OIDs should have
Peter Eisentraut <peter_e@gmx.net> 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é.
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.
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
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.
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
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
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.
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
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
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.
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
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
Josh Berkus <josh@agliodbs.com> 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
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.
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.
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
On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut <peter_e@gmx.net> 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 <module> 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
On Thu, Jul 4, 2013 at 2:18 AM, Hitoshi Harada <umi.tanuki@gmail.com> 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
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
On Mon, 2013-07-08 at 23:00 -0700, Hitoshi Harada wrote: > On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut <peter_e@gmx.net> > 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?
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
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.
Attachment
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.
Attachment
Hi, Peter Eisentraut <peter_e@gmx.net> 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 abetter 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 atleast) 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 transformfunctions. I agree with the need, just wanted to raise awereness about that in case there's something to besaid 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 changethe 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 reasonwhy 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
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 <type> <groupname> (...details...); and then there is SET DEFAULT TRANSFORM GROUP <groupname> SET TRANSFORM GROUP FOR TYPE <type> <groupname> 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 <type> LANGUAGE <lang> (...details...); So you could consider LANGUAGE <lang> to be the implicit transform group of language <lang>, 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.)
On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut <peter_e@gmx.net> 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
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 <peter_e@gmx.net> 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 usersinto using extensions, not good), - per function (hard to maintain), - something else. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
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Ü
On 11/20/2013 10:58 PM, Robert Haas wrote: > On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut <peter_e@gmx.net> 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Ü
On 11/15/2013 05:04 PM, Dimitri Fontaine wrote: > Hi, > > Peter Eisentraut <peter_e@gmx.net> 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 <xformname> FOR <type> LANGUAGE <lang> (...details...); 2) use it when declaring a function CREATE function <funcname>( IN <argname> <type> [[USING] [TRANSFORM] <xformname>], INOUT <argname> <type> [[USING] [IN][TRANSFORM] <xformname>] [[USING] [OUT] [TRANSFORM] <xformname>], OUT <argname> <type> [[USING] [TRANSFORM] <xformname>], ... ) LANGUAGE <lang> $$ <funcdef> $$; 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Ü
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.
Peter Eisentraut <peter_e@gmx.net> 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
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Ü
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.
Attachment
On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut <peter_e@gmx.net> 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
On 12/11/2013 01:40 PM, Robert Haas wrote: > On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut <peter_e@gmx.net> 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Ü
On Wed, Dec 11, 2013 at 9:19 AM, Hannu Krosing <hannu@2ndquadrant.com> wrote: > On 12/11/2013 01:40 PM, Robert Haas wrote: >> On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut <peter_e@gmx.net> 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
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut <peter_e@gmx.net> 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
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.
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.
On Fri, Jan 10, 2014 at 10:40 PM, Peter Eisentraut <peter_e@gmx.net> 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
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. ;-)
Attachment
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
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
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
Attachment
Peter Eisentraut <peter_e@gmx.net> 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
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 Regards, -- Michael
<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">On Mon, Dec 22, 2014 at 12:19 PM, Michael Paquier<span dir="ltr"><<a href="mailto:michael.paquier@gmail.com" target="_blank">michael.paquier@gmail.com</a>></span>wrote:<br /><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px#ccc solid;padding-left:1ex">On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut <<a href="mailto:peter_e@gmx.net">peter_e@gmx.net</a>>wrote:<br /> > fixed<br /> This patch needs a rebase, it does notapply correctly in a couple of<br /> places on latest HEAD (699300a):<br /> ./src/include/catalog/catversion.h.rej<br/> ./src/include/catalog/pg_proc.h.rej<br /> ./src/pl/plpython/plpy_procedure.c.rej<spanclass="HOEnZb"><font color="#888888"><br /></font></span></blockquote></div><br/></div><div class="gmail_extra">I moved this patch to 2015-02 to not lose track ofit and because it did not receive much reviews...<br />-- <br /><div class="gmail_signature">Michael<br /></div></div></div>
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
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.)
Attachment
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.)
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).
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
On Mon, Mar 16, 2015 at 9:51 PM, Peter Eisentraut <peter_e@gmx.net> 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
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.
Attachment
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
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
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
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.
Attachment
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
Attachment
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
Attachment
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. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
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.
On Wed, Apr 29, 2015 at 3:21 PM, Jeff Janes <jeff.janes@gmail.com> wrote: > 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. Cool. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company