Re: [PATCH] Add transforms feature

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.