Re: [PATCH] Add transforms feature - Mailing list pgsql-hackers

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