Re: [PATCH] Add transforms feature

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.