Re: transforms

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