2015-03-22 5:45 GMT+01:00 Pavel Stehule <pavel.steh...@gmail.com>: > > > 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 >
I am sorry, there is tests > > 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 > >
