Re: [PATCH] Add transforms feature

2015-04-08 4:55 GMT+02:00 Peter Eisentraut <peter_e(at)gmx(dot)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