2015-03-22 3:55 GMT+01:00 Peter Eisentraut <pete...@gmx.net>: > 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 <pete...@gmx.net > > <mailto:pete...@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. >
1. It add attribute to pg_proc, so impact is not minimal 2. Minimally it is not tested - there are no any test for this functionality 3. I'll reread a discuss about this design - Now I am thinking so this duality (in design) is wrong - worse in relatively critical part of Postgres. I can mark this patch as "ready for commiter" with objection - It is task for commiter, who have to decide. Regards Pavel
