Re: [HACKERS] [PATCH] Add transforms feature
On Tue, Apr 28, 2015 at 12:47 PM, Jeff Janes jeff.ja...@gmail.com wrote: This commit is causing a compiler warning for me in non-cassert builds: funcapi.c: In function 'get_func_trftypes': funcapi.c:890: warning: unused variable 'procStruct' Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it. I took a stab at fixing this via a slightly different method. Let me know whether that worked. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Tue, Apr 7, 2015 at 7:55 PM, Peter Eisentraut pete...@gmx.net wrote: 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. Hi Peter, This commit is causing a compiler warning for me in non-cassert builds: funcapi.c: In function 'get_func_trftypes': funcapi.c:890: warning: unused variable 'procStruct' Adding PG_USED_FOR_ASSERTS_ONLY seems to fix it. Cheers, Jeff transform_unused.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
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. ok, please, can comment the reason in test. little bit more verbose than make sure the prerequisite libraries are loaded. There is not clean, why LOAD should not be used. 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. So I reread discussion about this topic and I can see some benefits of it. Still - what I dislike is the behave of TRANSFORM ALL. The fact, so newly installed transformations are not used for registered functions (and reregistration is needed) is unhappy. I understand, so it can depends on implementation requirements. 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. Small issue - explicitly setted transformation types should be visible in \sf and \df+ commands. Regards Pavel
Re: [HACKERS] [PATCH] Add transforms feature
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
Re: [HACKERS] [PATCH] Add transforms feature
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
Re: [HACKERS] [PATCH] Add transforms feature
On Mon, Mar 16, 2015 at 9:51 PM, Peter Eisentraut pete...@gmx.net wrote: 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 think we need to the ability to control it per-function, but having a global disabling knob on top of that doesn't seem especially useful. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
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. 2. strange load lib by in sql scripts: DO '' LANGUAGE plperl; SELECT NULL::hstore; use load plperl; load hstore; instead OK 3. missing documentation for new contrib modules, OK 4. pg_dump - wrong comment +---/* +--- * protrftypes was added at v9.4 +--- */ OK 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. 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). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
2015-03-17 2:51 GMT+01:00 Peter Eisentraut 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. 2. strange load lib by in sql scripts: DO '' LANGUAGE plperl; SELECT NULL::hstore; use load plperl; load hstore; instead OK 3. missing documentation for new contrib modules, OK 4. pg_dump - wrong comment +---/* +--- * protrftypes was added at v9.4 +--- */ OK 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. 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. Regards Pavel
Re: [HACKERS] [PATCH] Add transforms feature
Hi I am looking to code. Some small issues: 1. fix missing semicolon pg_proc.h Oid protrftypes[1]; /* types for which to apply transforms */ 2. strange load lib by in sql scripts: DO '' LANGUAGE plperl; SELECT NULL::hstore; use load plperl; load hstore; instead 3. missing documentation for new contrib modules, 4. pg_dump - wrong comment +---/* +--- * protrftypes was added at v9.4 +--- */ 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. 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. Regards Pavel 2015-03-08 16:34 GMT+01:00 Peter Eisentraut pete...@gmx.net: On 3/6/15 9:56 AM, Pavel Stehule wrote: Hi I am checking this patch, but it is broken still Here is an updated patch. (Note that because of the large pg_proc.h changes, it is likely to get outdated again soon. But for reviewing, you can always apply it against an older version of master.)
Re: [HACKERS] [PATCH] Add transforms feature
Hi I am checking this patch, but it is broken still Regards Pavel 2015-02-13 8:14 GMT+01:00 Michael Paquier michael.paqu...@gmail.com: On Mon, Dec 22, 2014 at 12:19 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut pete...@gmx.net wrote: fixed This patch needs a rebase, it does not apply correctly in a couple of places on latest HEAD (699300a): ./src/include/catalog/catversion.h.rej ./src/include/catalog/pg_proc.h.rej ./src/pl/plpython/plpy_procedure.c.rej I moved this patch to 2015-02 to not lose track of it and because it did not receive much reviews... -- Michael
Re: [HACKERS] [PATCH] Add transforms feature
On Mon, Dec 15, 2014 at 3:10 PM, Peter Eisentraut pete...@gmx.net wrote: fixed This patch needs a rebase, it does not apply correctly in a couple of places on latest HEAD (699300a): ./src/include/catalog/catversion.h.rej ./src/include/catalog/pg_proc.h.rej ./src/pl/plpython/plpy_procedure.c.rej Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter Eisentraut pete...@gmx.net writes: On 4/4/14 6:21 PM, Andres Freund wrote: + /* dependency on transform used by return type, if any */ + if ((trfid = get_transform_oid(returnType, languageObjectId, true))) Should be compared to InvalidOid imo, rather than implicitly assuming that InvalidOid evaluates to false. I think it's widely assumed that InvalidOid is false. That's not the point; the point is that some nonzero number of compilers (and probably Coverity too) will warn about this construct, on the not unreasonable grounds that = might be a typo for ==. (Those extra parens might satisfy gcc, but not other tools.) Please put in an explicit comparison of the assignment result, as is done in approximately 100% of the other places where this idiom appears in Postgres. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 2014-04-05 00:21:47 +0200, Andres Freund wrote: On 2014-01-15 21:13:18 -0500, Peter Eisentraut wrote: The attached patch will probably fail to apply because of the pg_proc changes. So if you want to try it out, look into the header for the Git hash it was based off. I'll produce a properly merged version when this approach is validated. Also, some implementation details could probably take some revising after a couple of nights of sleep. ;-) You've slept since? ;) So, I am only doign a look through the patch, to see where it has gone in the past year. ... So, unless somebody protests PDQ, I am going to mark this Returned with Feedback. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 2014-01-15 21:13:18 -0500, Peter Eisentraut wrote:
The attached patch will probably fail to apply because of the pg_proc
changes. So if you want to try it out, look into the header for the Git
hash it was based off. I'll produce a properly merged version when this
approach is validated. Also, some implementation details could probably
take some revising after a couple of nights of sleep. ;-)
You've slept since? ;)
So, I am only doign a look through the patch, to see where it has gone
in the past year.
index 4e476c3..5ac9f05 100644
--- a/src/Makefile.shlib
+++ b/src/Makefile.shlib
@@ -133,7 +133,7 @@ ifeq ($(PORTNAME), darwin)
else
# loadable module
DLSUFFIX = .so
-LINK.shared = $(COMPILER) -bundle -multiply_defined suppress
+LINK.shared = $(COMPILER) -bundle -multiply_defined
suppress -Wl,-undefined,dynamic_lookup
endif
BUILD.exports = $(AWK) '/^[^\#]/ {printf _%s\n,$$1}' $ $@
exports_file = $(SHLIB_EXPORTS:%.txt=%.list)
Hm. Do the linkers on other platforms support this behaviour? Linux
does, by default, but I have zap clue about the rest.
Why do we need this? I guess because a transform from e.g. hstore to $pl
needs symbols out of hstore.so and the $pl?
I wonder if it woudln't be better to rely on explicit symbol lookups for
this. That'd avoid the need for the order dependency and linker changes
like this.
+ case OBJECT_TRANSFORM:
+ {
+ TypeName *typename = (TypeName *)
linitial(objname);
+ char *langname = (char *)
linitial(objargs);
+ Oid type_id =
typenameTypeId(NULL, typename);
+ Oid lang_id =
get_language_oid(langname, false);
+
+ address.classId = TransformRelationId;
+ address.objectId =
+ get_transform_oid(type_id,
lang_id, missing_ok);
+ address.objectSubId = 0;
+ }
+ break;
Hm. I wonder if missing_ok should be forwarded to get_language_oid() and
(by changing the way things are done atm) to typenameTypeId?
+ case OCLASS_TRANSFORM:
+ {
+ HeapTuple trfTup;
+ Form_pg_transform trfForm;
+
+ trfTup = SearchSysCache1(TRFOID,
+
ObjectIdGetDatum(object-objectId));
+ if (!HeapTupleIsValid(trfTup))
+ elog(ERROR, could not find tuple for
transform %u,
+ object-objectId);
+
+ trfForm = (Form_pg_transform) GETSTRUCT(trfTup);
+
+ appendStringInfo(buffer, _(transform for %s
language %s),
+
format_type_be(trfForm-trftype),
+
get_language_name(trfForm-trflang, false));
+
+ ReleaseSysCache(trfTup);
+ break;
+ }
+
Why deviate from the usual 'cache lookup failed for ..'? elog doesn't
translate so it's not particular relevant, but ...
referenced.objectSubId = 0;
recordDependencyOn(myself, referenced, DEPENDENCY_NORMAL);
+ /* dependency on transform used by return type, if any */
+ if ((trfid = get_transform_oid(returnType, languageObjectId, true)))
+ {
+ referenced.classId = TransformRelationId;
+ referenced.objectId = trfid;
+ referenced.objectSubId = 0;
+ recordDependencyOn(myself, referenced, DEPENDENCY_NORMAL);
+ }
+
Should be compared to InvalidOid imo, rather than implicitly assuming
that InvalidOid evaluates to false.
+/*
+ * CREATE TRANSFORM
+ */
+Oid
+CreateTransform(CreateTransformStmt *stmt)
+{
...
+ if (!pg_type_ownercheck(typeid, GetUserId()))
+ aclcheck_error_type(ACLCHECK_NOT_OWNER, typeid);
+
+ aclresult = pg_type_aclcheck(typeid, GetUserId(), ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error_type(aclresult, typeid);
+
+ /*
+ * Get the language
+ */
+ langid = get_language_oid(stmt-lang, false);
+
+ aclresult = pg_language_aclcheck(langid, GetUserId(), ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_LANGUAGE, stmt-lang);
Hm. Is USAGE really sufficient here? Should we possibly make it
dependant on
Re: [HACKERS] [PATCH] Add transforms feature
On Fri, Jan 10, 2014 at 10:40 PM, Peter Eisentraut pete...@gmx.net wrote: On Wed, 2013-12-11 at 11:07 -0500, Tom Lane wrote: We should have learned by now that those are usually a bad idea. In this case, we've got changes in the behavior of function calling, which seems like not only a nightmare for debugging but a fertile source of security issues. I note that this is the same mechanism that we have elaborately designed for *avoiding* security issues from search_path. And it works like crap. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Wed, 2013-12-11 at 11:07 -0500, Tom Lane wrote: We should have learned by now that those are usually a bad idea. In this case, we've got changes in the behavior of function calling, which seems like not only a nightmare for debugging but a fertile source of security issues. I note that this is the same mechanism that we have elaborately designed for *avoiding* security issues from search_path. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Wed, 2013-12-11 at 09:47 -0500, Robert Haas wrote: Of course, making everyone decorate their new functions with references to the transforms they want to use isn't wonderful either, but it might be good at least to have the option. You could allow the use of all installed transforms by default, but let people say WITH (transforms='') if they don't want to use them or WITH (transforms='comma, separated, list') if the want to require certain ones. I'll try to implement something like that. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut pete...@gmx.net wrote: On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. Here is an updated patch that implements this, makes some of the documentation improvements that you suggested, and rebases everything. I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 12/11/2013 01:40 PM, Robert Haas wrote: On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut pete...@gmx.net wrote: On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. Here is an updated patch that implements this, makes some of the documentation improvements that you suggested, and rebases everything. I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. It should work ok if we could somehow check that the GUC is set on the function and fall back to session GUC in case it is not. Not sure if this is possible though. The need from this arises from calling other functions from a new func. At the moment if there is a new function defined as CREATE FUNCTION f_uses_xforms() AS $$ ... $$ SET use_transforms=on; calls a legacy function which will break if transforms are used then the _old_ function declaration needs to be modified to add (use_transforms=off) It is much easier than debugging/rewriting the function, but this is something I'd like us to be able to avoid. PS. maybe we could resurrect the WITH (attribute, ...) available in CREATE FUNCTION syntax for passing function-specific flags ? Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Wed, Dec 11, 2013 at 9:19 AM, Hannu Krosing ha...@2ndquadrant.com wrote: On 12/11/2013 01:40 PM, Robert Haas wrote: On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut pete...@gmx.net wrote: On Fri, 2013-12-06 at 11:28 +0100, Dimitri Fontaine wrote: Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. Here is an updated patch that implements this, makes some of the documentation improvements that you suggested, and rebases everything. I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. It should work ok if we could somehow check that the GUC is set on the function and fall back to session GUC in case it is not. Not sure if this is possible though. The need from this arises from calling other functions from a new func. At the moment if there is a new function defined as CREATE FUNCTION f_uses_xforms() AS $$ ... $$ SET use_transforms=on; calls a legacy function which will break if transforms are used then the _old_ function declaration needs to be modified to add (use_transforms=off) Yeah, exactly. It is much easier than debugging/rewriting the function, but this is something I'd like us to be able to avoid. PS. maybe we could resurrect the WITH (attribute, ...) available in CREATE FUNCTION syntax for passing function-specific flags ? It's a thought. Or you could put some annotation in the function body, as we do in PL/pgsql with the #option syntax. Of course, making everyone decorate their new functions with references to the transforms they want to use isn't wonderful either, but it might be good at least to have the option. You could allow the use of all installed transforms by default, but let people say WITH (transforms='') if they don't want to use them or WITH (transforms='comma, separated, list') if the want to require certain ones. Unfortunately, that'll probably mean that virtually all portable code for procedural languages has to include some form of this incantation, just as any nearly any PL/pgsql function has to include SET search_path = '' if it wants to be not trivially subvertable. It's annoying to grow more such decoration, but the alternative seems to be hoping that nobody wants to write portable code that uses non-core types, and that doesn't seem better. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Robert Haas robertmh...@gmail.com writes: On Tue, Dec 10, 2013 at 10:35 PM, Peter Eisentraut pete...@gmx.net wrote: Here is an updated patch that implements this, makes some of the documentation improvements that you suggested, and rebases everything. I'm still kinda unimpressed by this. Behavior-changing GUC, uggh. We should have learned by now that those are usually a bad idea. In this case, we've got changes in the behavior of function calling, which seems like not only a nightmare for debugging but a fertile source of security issues. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter Eisentraut pete...@gmx.net writes: Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. +1 The only thing this doesn't give you is per-argument granularity, but I think the use cases for that are slim, and we don't have a good existing mechanism to attach arbitrary attributes to function arguments. +1 Actually, I'd take this two steps further. First, make this parameter per-language, so something like plpython.use_transforms. Then it's up to the language implementation how they want to deal with this. A future new language could just ignore the whole issue and require transforms from the start. I'm not sure about this level of granularity, but why not. Second, depending on the choice of the language, this parameter could take three values: ignore | if available | require. That would allow users to set various kinds of strictness, for example if they want to be alerted that a language cannot deal with a particular type. My understanding is that it always can deal with any particular type if you consider text based input/output, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 12/06/2013 07:25 AM, Peter Eisentraut wrote: On Tue, 2013-11-26 at 18:10 +0100, Dimitri Fontaine wrote: The problem is installing a set of extensions where some of them are already using the new transform feature and some of them are not. We need a way to cater with that, I think. Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. It would requite the old extensions to be modified to have (SET use_transforms = off) in all their definitions so that they would not accidentally be called with use_transforms = on from new functions, but else it seems like a good way to get it done without too much effort. The only thing this doesn't give you is per-argument granularity, but I think the use cases for that are slim, and we don't have a good existing mechanism to attach arbitrary attributes to function arguments. Agreed. And we are quite unlikely to need multiple transforms for the same type in the same language. Actually, I'd take this two steps further. First, make this parameter per-language, so something like plpython.use_transforms. Then it's up to the language implementation how they want to deal with this. A future new language could just ignore the whole issue and require transforms from the start. I do not really see much need for this, as it will need to be set for each individual function anyway. Actually what we could do is just declare a new language for this so functions declared with LANGUAGE plpythonu will not be using transforms and those with LANGUAGE plpythonuxf will use it. This would only need one extra function to be defined in source code, namely the compile function for the new language. Some not-transforms-related wild ideas follow :) Adding a new language would also be a good way to fix the bad syntax choices in pl/python which require code manipulation before compiling . I came up with this idea after seeing how pl/jsv8 supports multiple JavaScript-based languages (standard JavaScript, CoffeeScript, LiveScript) from the same codebase. Taking the plv8 ideas further we could also create a JavaScript-based sandboxed python using thins like skulpt and pyjamas which compile python source code to JavaScript VM and inherit all the sandboxing of v8. Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Tue, 2013-11-26 at 18:10 +0100, Dimitri Fontaine wrote: The problem is installing a set of extensions where some of them are already using the new transform feature and some of them are not. We need a way to cater with that, I think. Here is an idea. Add a GUC that basically says something like use_transforms = on|off. You can then attach that to individual functions, which is the right granularity, because only the function knows whether its code expects transforms or not. But you can use the full power of GUC to configure it any way you want. The only thing this doesn't give you is per-argument granularity, but I think the use cases for that are slim, and we don't have a good existing mechanism to attach arbitrary attributes to function arguments. Actually, I'd take this two steps further. First, make this parameter per-language, so something like plpython.use_transforms. Then it's up to the language implementation how they want to deal with this. A future new language could just ignore the whole issue and require transforms from the start. Second, depending on the choice of the language, this parameter could take three values: ignore | if available | require. That would allow users to set various kinds of strictness, for example if they want to be alerted that a language cannot deal with a particular type. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 11/15/2013 05:04 PM, Dimitri Fontaine wrote: Hi, Peter Eisentraut pete...@gmx.net writes: Rebased patch. No changes except that merge conflicts were resolved, and I had to add some Data::Dumper tweaks to the regression tests so that the results came out in consistent order on different versions of Perl. On the higher level design, the big question here is about selective behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl then any plperl function will now receive its hstore arguments as a proper perl hash rather than a string. Any pre-existing plperl function with hstore arguments or return type then needs to be upgraded to handle the new types nicely, and some of those might not be under the direct control of the DBA running the CREATE TRANSFORM command, when using some plperl extensions for example. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. If we start adding granularity, then why not go all the way? I mean, we could do it in the following way 1) create named transforms CREATE [DEFAULT] TRANSFORM xformname FOR type LANGUAGE lang (...details...); 2) use it when declaring a function CREATE function funcname( IN argname type [[USING] [TRANSFORM] xformname], INOUT argname type [[USING] [IN] [TRANSFORM] xformname] [[USING] [OUT] [TRANSFORM] xformname], OUT argname type [[USING] [TRANSFORM] xformname], ... ) LANGUAGE lang $$ funcdef $$; This approach allows full flexibility in using old packages, especially if we define old transform behaviour as DEFAULT TRANSFORM Default transforms also allow easy way for rewriting current type i/o conversions between languages into transforms. There are immediately a few transforms that I would find useful A) pass field data to language as pairs of (typeoid, typebin) this is useful for speed, especially if you do not want to use many of the passed arguments on most invocations B) pass field data in as (typeoid, typebin), except do not de-toast values but pass in the toast ids, so the function is free to use only parts of toasted values as it needs C) pass field data in as string, probably the default behaviour for languages like pl/tcl and pl/sh D) and then of course just having a sensible transforms for extension types like the current patch provides. Worst case, that I really don't think we need, would be addressing that per-argument: CREATE FUNCTION foo (hash hstore WITH TRANSFORM, kv hstore) … I certainly hope we don't need that, and sure can't imagine use cases for that level of complexity at the time of writing this review. A typical use case would be to have a short hstore always passed in as dictionary and have another possibly large hstore passed in as toast pointer. And if we want to have all type conversions between postgres and pls re-written as transforms, then we do need named transforms, not just one per (pl, type) pair. Also, if we allow flexibility, the it is probably a good idea to implement full flexibility first and then look at making usage easy after that, instead of adding flexibility in small steps. Once we have per-argument transforms in place, we can look at setting per-schema defaults for ease of use. As large part of this is actually abstracting i/o conversions out of pl function code, I think we should look at allowing the conversion functions to be written in the target pl language in addition to C. I'll see if I can resurrect my patch for support of cstring and internal types in pl/python function defs for this. -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Hi, Allow me to temporarily skip important questions that you asked so that we can focus on the main problem here. As soon as we decide how to handle any kind of selectivity for the transforms, then I'm back to answering the other things. Peter Eisentraut pete...@gmx.net writes: Let's review that, as there as been some confusion about that. The SQL standard syntax is Thanks for that. I see that you reused parts of the concept and keywords and implemented something specific to PostgreSQL from there. As there's no collision within the command sets, I think that problem is cleared. I had proposed disallowing installing a transform that would affect existing functions. That was rejected or deemed unnecessary. You can't have it both ways. ;-) Well I'm not so sure, that's the point. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. Anything that's a problem per-database would also be a problem per schema. But a smaller problem, and as problem get smaller they get easier to reason about and fix too. In the example given by Robert Haas in the same thread, you could install extension A in schema A using the transforms for hstore and plperl and extension B in schema B not using the same transforms. I'm not saying using the schema that way is awesome, just that we have solid precedent in the standard and in pljava, and it looks easy enough to implement (we already have search_path invalidations IIRC). This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. I disagree. How many people actually use hstore with PL/Perl or PL/Python now? Probably not many, because it's weird. I like to think about how this works for new development: Here is my extension type, here is how it interfaces with languages. Once you have established that, you don't want to have to repeat that every time you write a function. That's error prone and cumbersome. And anything that's set per schema or higher is a dependency tracking and caching mess. The problem is installing a set of extensions where some of them are already using the new transform feature and some of them are not. We need a way to cater with that, I think. Also, extension types should work the same as built-in types. Eventually, I'd like to rip out the hard-coded data type support in PL/Python and replace it with built-in transforms. Even if we don't actually do it, conceptually it should be possible. Now if we require I like that idea a lot. I don't see how you can make it work by default, as like Robert I think the transition phase you're talking about will never end. USING TRANSFORM FOR int, bytea every time, we'd have taken a big step back. Effectively, we already have built-in transforms in PL/Python. For core types only. We have added a few more over the years. It's been a bit of a pain from time to time. At least, with this feature we'd be moving this decision into user space and give people a way to fix things. (Incidentally, if you add a lot of transforms, you are probably dealing with a strongly typed language. And a strongly typed language is more likely to cleanly catch type errors resulting from changes in the transforms.) I think use space will want to be able to use code written using different sets of transforms for the same set of types, rather than being forced into upgrading their whole code at once. It reminds me of the python 2 to python 3 upgrade path. This is not solved yet, with libs that are python2 only and others that are python3 only, and OS that would ship some but not others. We already have the problem that shipping contrib by default is frowned upon at some places, then they miss out of plenty of awesome extensions. Transforms with no granularity is only going to make it worse, I think. So we have to choose the granularity: - per database (current patch), - per schema (standard has precedents with pljava, - per extension (forcing users into using extensions, not good), - per function (hard to maintain), - something else. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 11/12/2013 12:21 PM, Peter Eisentraut wrote: A transform is an SQL object that supplies to functions for converting between data types and procedural languages. How hard would it be to extend this to add transforms directly between pairs of procedural languages ? One example would be calling a pl/v8 function from pl/python and converting directly between integers in both, without going through PostgreSQL type. Another and maybe even more interesting would be automatic null-transforms between two pl/python functions. Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 11/20/2013 10:58 PM, Robert Haas wrote: On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut pete...@gmx.net wrote: This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. Sure, but that's like saying nobody's required to use this behavior-changing GUC, so it's OK to have a behavior-changing GUC. The point I think Dimitri is making, which IMHO is entirely valid, is that the feature as currently designed is database-wide. You either get this behavior for all of your functions, or you get it for none of them, and that might well not be what you want. For example, it's easy to imagine that you might want to install extensions A and B. A expects that a certain transform is loaded into the database, and B expects that it isn't. You now have created a situation where extensions A and B can't be used together. That sucks. If the transform were a property of particular function argument positions, this wouldn't be a problem. You could declare, in effect, that a certain function takes a transformed hstore, and this other one takes a non-transformed hstore. Now life is good. But that's not what is being proposed. You mean something like CREATE FUNCTION f(i int, h1 hstore USING TRANSFORM x, h2 hstore) ... where h1 would go through transform x and 1 and h2 would use default transform ? Cheers -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 11/15/13, 11:04 AM, Dimitri Fontaine wrote: - Documentation style seems to be to be different from the man page or reference docs style that we use elsewhere, and is instead deriving the general case from examples. Reads strange. Which specific section do you have in mind? It's hard to explain this feature in abstract terms, I think. - The internal datatype argument and return type discussion for function argument looks misplaced, but I don't have a better proposition for that. OK, maybe I'll put that in parentheses or a separate paragraph. - Do we need an ALTER TRANSFORM command? Usually we have at least an Owner for the new objects and a command to change the owner. Then should we be able to change the function(s) used in a transform? We don't have ALTER CAST either, and no one's been too bothered about that. It's possible, of course. - Should transform live in a schema? At first sight, no reason why, but see next point about a use case that we might be able to solve doing that. Transforms don't have a name, so I don't quite see what you mean here. - SQL Standard has something different named the same thing, targetting client side types apparently. Is there any reason why we would want to stay away from using the same name for something really different in PostgreSQL? Let's review that, as there as been some confusion about that. The SQL standard syntax is CREATE TRANSFORM FOR type groupname (...details...); and then there is SET DEFAULT TRANSFORM GROUP groupname SET TRANSFORM GROUP FOR TYPE type groupname This is essentially an elaborate way to have custom input/output formats, like DateStyle or bytea_output. (You can find examples of this in the IBM DB2 documentation. Some of their clients apparently set a certain transform group automatically, allowing you to set per-interface output formats.) The proposed syntax in the other hand is CREATE TRANSFORM FOR type LANGUAGE lang (...details...); So you could consider LANGUAGE lang to be the implicit transform group of language lang, if you like. Or you could consider that this is a situation like VIEW vs. MATERERIALIZED VIEW: they sound the same, they are a bit alike, but the implementation details are different. All obvious synonyms of transform (conversion, translation, etc.) are already in use. On the higher level design, the big question here is about selective behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl then any plperl function will now receive its hstore arguments as a proper perl hash rather than a string. Any pre-existing plperl function with hstore arguments or return type then needs to be upgraded to handle the new types nicely, and some of those might not be under the direct control of the DBA running the CREATE TRANSFORM command, when using some plperl extensions for example. I had proposed disallowing installing a transform that would affect existing functions. That was rejected or deemed unnecessary. You can't have it both ways. ;-) A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. Anything that's a problem per-database would also be a problem per schema. Should using the schema to that ends be frowned upon, then we need a way to register each plperl function against using or not using the transform facility, defaulting to not using anything. Maybe something like the following: CREATE FUNCTION foo(hash hstore, x ltree) RETURNS hstore LANGUAGE plperl USING TRANSFORM FOR hstore, ltree AS $$ … $$; This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. How many people actually use hstore with PL/Perl or PL/Python now? Probably not many, because it's weird. I like to think about how this works for new development: Here is my extension type, here is how it interfaces with languages. Once you have established that, you don't want to have to repeat that every time you write a function. That's error prone and cumbersome. And anything that's set per schema or higher is a dependency tracking and caching mess. Also, extension types should work the same as built-in types. Eventually, I'd like to rip out the hard-coded data type support in PL/Python and replace it with built-in transforms. Even if we don't actually do it, conceptually it should be possible. Now if we require USING TRANSFORM FOR int, bytea every time, we'd have taken a big step back. Effectively, we already have built-in transforms in PL/Python. We have added a few more over the years. It's been a bit of a pain from time to time. At least, with this feature we'd be moving this decision into user space and give
Re: [HACKERS] [PATCH] Add transforms feature
On Wed, Nov 20, 2013 at 11:51 AM, Peter Eisentraut pete...@gmx.net wrote: This is a transition problem. Nobody is required to install the transforms into their existing databases. They probably shouldn't. Sure, but that's like saying nobody's required to use this behavior-changing GUC, so it's OK to have a behavior-changing GUC. The point I think Dimitri is making, which IMHO is entirely valid, is that the feature as currently designed is database-wide. You either get this behavior for all of your functions, or you get it for none of them, and that might well not be what you want. For example, it's easy to imagine that you might want to install extensions A and B. A expects that a certain transform is loaded into the database, and B expects that it isn't. You now have created a situation where extensions A and B can't be used together. That sucks. If the transform were a property of particular function argument positions, this wouldn't be a problem. You could declare, in effect, that a certain function takes a transformed hstore, and this other one takes a non-transformed hstore. Now life is good. But that's not what is being proposed. You could argue that such a level of granularity is overkill, but frankly I've never had a real good feeling about the likelihood of these transforms getting installed in the first place. In theory, if you're using hstore and you're using plperl, you ought to also install hstore-plperl-transform, but I bet a significant percentage of people won't. So I don't foresee a finite transition period after which databases without transforms go away and all code is written with the assumption that transforms are available; instead, I foresee different people assuming different things and ending up with mutually incompatible code bases. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Hi, Peter Eisentraut pete...@gmx.net writes: Rebased patch. No changes except that merge conflicts were resolved, and I had to add some Data::Dumper tweaks to the regression tests so that the results came out in consistent order on different versions of Perl. I just spent some time reading that patch, here's my first round of review: - Documentation style seems to be to be different from the man page or reference docs style that we use elsewhere, and is instead deriving the general case from examples. Reads strange. - The internal datatype argument and return type discussion for function argument looks misplaced, but I don't have a better proposition for that. - The code looks good, I didn't spot any problem when reading it. Given your Jenkins installation I didn't try (yet at least) to use the patch and compile and run it locally. Interesting note might be the addition of two new system caches, one for the PL languages and another one for the transform functions. I agree with the need, just wanted to raise awereness about that in case there's something to be said on that front. - Do we need an ALTER TRANSFORM command? Usually we have at least an Owner for the new objects and a command to change the owner. Then should we be able to change the function(s) used in a transform? - Should transform live in a schema? At first sight, no reason why, but see next point about a use case that we might be able to solve doing that. - SQL Standard has something different named the same thing, targetting client side types apparently. Is there any reason why we would want to stay away from using the same name for something really different in PostgreSQL? On the higher level design, the big question here is about selective behavior. As soon as you CREATE TRANSFORM FOR hstore LANGUAGE plperl then any plperl function will now receive its hstore arguments as a proper perl hash rather than a string. Any pre-existing plperl function with hstore arguments or return type then needs to be upgraded to handle the new types nicely, and some of those might not be under the direct control of the DBA running the CREATE TRANSFORM command, when using some plperl extensions for example. A mechanism allowing for the transform to only be used in some functions but not others might be useful. The simplest such mechanism I can think of is modeled against the PL/Java classpath facility as specified in the SQL standard: you attach a classpath per schema. We could design transforms to only activate in the schema they are created, thus allowing plperl functions to co-exist where some will receive proper hash for hstores and other will continue to get plain text. Should using the schema to that ends be frowned upon, then we need a way to register each plperl function against using or not using the transform facility, defaulting to not using anything. Maybe something like the following: CREATE FUNCTION foo(hash hstore, x ltree) RETURNS hstore LANGUAGE plperl USING TRANSFORM FOR hstore, ltree AS $$ … $$; Worst case, that I really don't think we need, would be addressing that per-argument: CREATE FUNCTION foo (hash hstore WITH TRANSFORM, kv hstore) … I certainly hope we don't need that, and sure can't imagine use cases for that level of complexity at the time of writing this review. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: CREATE TRANSFORM syntax (was Re: [HACKERS] [PATCH] Add transforms feature)
On 08/13/2013 07:16 PM, Peter Eisentraut wrote: My next best idea is CREATE TRANSFORM FOR hstore SERVER LANGUAGE plperl, which preserves the overall idea but still distinguishes server from client languages. Comments? My thinking is that TRANSFORMS will almost certainly be managed by installer/puppet scripts by users, so it doesn't really matter how ugly the syntax is, as long as it's unambiguous. Which is a roundabout way of saying whatever syntax you implement is fine with me from a usability perspective. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
CREATE TRANSFORM syntax (was Re: [HACKERS] [PATCH] Add transforms feature)
On Mon, 2013-07-08 at 23:00 -0700, Hitoshi Harada wrote: On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut pete...@gmx.net wrote: 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. I like it better than the current one. I had started to work on making this adjustment, but found the result very ugly. It also created a confusing association with CREATE SERVER, which is something different altogether. My next best idea is CREATE TRANSFORM FOR hstore SERVER LANGUAGE plperl, which preserves the overall idea but still distinguishes server from client languages. Comments? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 07/08/2013 12:00 PM, Josh Berkus wrote: (b) we can expect maybe a dozen to 18 of them in core based on the data types there, and I hate to clutter up /contrib, and Well, that's a matter of opinion. I'd be more happy with 250 contribs all on the same level versus a bunch of subdirectories structured based on personal preferences. But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) Yeah, I'd like to see some other opinions on this. Well, based on the total lack of opinions from anyone on -hackers, I'd say we leave your patch alone and leave the transforms in their current directory location. We can always move them later. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Sun, Jul 7, 2013 at 12:06 PM, Peter Eisentraut pete...@gmx.net wrote:
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.
I like it better than the current one.
- 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.
OK, my idea was worse, because the symbol resolution happens before
_PG_init anyway. But what I feel is, why can't we load dependency
libraries automatically? plpython can load libpython automatically, I
guess. Is it only the matter of LD_LIBRARY_PATH stuff?
- 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.
Yeah, I think it's better to limit to superuser.
- 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.
Here is the full set of what's happening.
test=# create extension hstore;
CREATE EXTENSION
test=# create extension plpython2u;
CREATE EXTENSION
test=# CREATE FUNCTION hstore_to_plpython2(val internal) RETURNS internal
test-# LANGUAGE C STRICT IMMUTABLE
test-# AS '$libdir/hstore_plpython2', 'hstore_to_plpython';
CREATE FUNCTION
test=#
test=# CREATE FUNCTION plpython2_to_hstore(val internal) RETURNS hstore
test-# LANGUAGE C STRICT IMMUTABLE
test-# AS 'hstore_plpython2', 'plpython_to_hstore';
CREATE FUNCTION
test=#
test=# CREATE TRANSFORM FOR hstore LANGUAGE plpython2u (
test(# FROM SQL WITH FUNCTION hstore_to_plpython2(internal),
test(# TO SQL WITH FUNCTION plpython2_to_hstore(internal)
test(# );
CREATE TRANSFORM
test=# create or replace function f(h hstore) returns hstore as $$
test$# h['b'] = 10
test$# return h
test$# $$ language plpython2u;
CREATE FUNCTION
test=# select f('a=1');
f
-
a=1, b=10
(1 row)
test=# CREATE OR REPLACE TRANSFORM FOR hstore LANGUAGE plpython2u (
test(# TO SQL WITH FUNCTION plpython2_to_hstore(internal)
test(# );
CREATE TRANSFORM
test=# select f('a=1');
f
-
a=1, b=10
(1 row)
test=# \c
You are now connected to database test as user haradh1.
test=# select f('a=1');
ERROR: TypeError: 'str' object does not support item assignment
CONTEXT: Traceback (most recent call last):
PL/Python function f, line 2, in module
h['b'] = 10
PL/Python function f
After reconnecting to the database with \c, the function behavior
changed because we did CREATE OR REPLACE. If we go with dependency
between function and transform, we shouldn't support OR REPLACE, I
guess. plpython can throw away the cache, but I feel like not
supporting OR REPLACE in this case.
--
Hitoshi Harada
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To
Re: [HACKERS] [PATCH] Add transforms feature
On Thu, Jul 4, 2013 at 2:18 AM, Hitoshi Harada umi.tan...@gmail.com wrote: For now, that's it. I'm going to dig more later. After looking into rest of the change, - TYPTYPE_DOMAIN is not supported. Why did you specifically disallow it? - ParseFuncOrColumn now prohibits to find function returning internal, but is it effective? I think it's already disallowed, or there is no way to call it. - get_transform_oid and get_transform are redundant -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter, On 07/07/2013 12:06 PM, Peter Eisentraut wrote: 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. Have we added the ability of non-superusers to create extensions yet? Until we have that, there's no point in allowing them to load transforms. Once we have that, we'll need to let them do it. 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. I don't think it's desirable. It would be hard to do, and at some level we need to make a presumption of competence on the part of the DBA. We should put a warning in the docs, though. (b) we can expect maybe a dozen to 18 of them in core based on the data types there, and I hate to clutter up /contrib, and Well, that's a matter of opinion. I'd be more happy with 250 contribs all on the same level versus a bunch of subdirectories structured based on personal preferences. But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) Yeah, I'd like to see some other opinions on this. (c) I'd like to do a future feature which supports install all transforms functionality, which would be helped by having them in their own directory. Installing all transforms by itself is not a sensible operation, because you only want the transforms for the types and languages that you actually use or have previously selected for installation. Give me some credit. I'm talking about a script for install all transforms for which the dependancies are already installed. That's certainly entirely doable, and would be made easier by putting the transforms in their own directory or otherwise flagging them to identify them as transforms. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Fri, 2013-07-05 at 12:04 -0700, Josh Berkus wrote: (a) transforms aren't like other contribs, in that they are dependant on other contribs before you install them. That doesn't appear to be a reason for creating subdirectories. (b) we can expect maybe a dozen to 18 of them in core based on the data types there, and I hate to clutter up /contrib, and Well, that's a matter of opinion. I'd be more happy with 250 contribs all on the same level versus a bunch of subdirectories structured based on personal preferences. But hey, we disagreed on config.sgml for similar reasons, IIRC. ;-) (c) I'd like to do a future feature which supports install all transforms functionality, which would be helped by having them in their own directory. Installing all transforms by itself is not a sensible operation, because you only want the transforms for the types and languages that you actually use or have previously selected for installation. I understand that this situation is unsatisfactory. But I don't think any dependency or package management system has solved it. For example, can any package management system make the following decision: user install PHP, user installs PostgreSQL client = install php-pgsql automatically. I don't think so. The only solutions are making PHP dependent on PostgreSQL (or vice versa), or having the user install php-pgsql explicitly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Josh Berkus j...@agliodbs.com writes: (c) I'd like to do a future feature which supports install all transforms functionality, which would be helped by having them in their own directory. I think we should install required extensions automatically when they are available. Also, we will need better tooling to deal with extension dependencies, see my patch for that from some time ago. https://commitfest.postgresql.org/action/patch_view?id=727 Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 7/3/13 7:15 PM, Josh Berkus wrote: I'm not comfortable with having all of the transform mappings in the main contrib/ directory though. Can we add a subdirectory called transforms containing all of these? I don't see any value in that. The data types they apply to are in contrib after all. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Friday, July 5, 2013, Peter Eisentraut wrote: On 7/3/13 7:15 PM, Josh Berkus wrote: I'm not comfortable with having all of the transform mappings in the main contrib/ directory though. Can we add a subdirectory called transforms containing all of these? I don't see any value in that. The data types they apply to are in contrib after all. I guess his suggestion is contrib/transforms directory, not transforms directory at top level. Stil you don't see value? -- Hitoshi Harada
Re: [HACKERS] [PATCH] Add transforms feature
On 07/05/2013 09:08 AM, Peter Eisentraut wrote: On 7/3/13 7:15 PM, Josh Berkus wrote: I'm not comfortable with having all of the transform mappings in the main contrib/ directory though. Can we add a subdirectory called transforms containing all of these? I don't see any value in that. The data types they apply to are in contrib after all. Well, I had three thoughts on this: (a) transforms aren't like other contribs, in that they are dependant on other contribs before you install them. (b) we can expect maybe a dozen to 18 of them in core based on the data types there, and I hate to clutter up /contrib, and (c) I'd like to do a future feature which supports install all transforms functionality, which would be helped by having them in their own directory. That's my thinking on it anyway. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On Thu, Jun 13, 2013 at 8:11 PM, Peter Eisentraut pete...@gmx.net wrote: A transform is an SQL object that supplies to functions for converting between data types and procedural languages. For example, a transform could arrange that hstore is converted to an appropriate hash or dictionary object in PL/Perl or PL/Python. The patch applies and regression including contrib passes in my Mac. I know I came late, but I have a few questions. - vs SQL standard I'm worried about overloading the standard. As document says, SQL standard defines CREATE TRANSFORM syntax which is exactly the same as this proposal but for different purpose. The standard feature is the data conversion between client and server side data type, I guess. I am concerned about it because in the future if someone wants to implement this SQL standard feature, there is no way to break thing. I'd be happy if subsequent clause was different. CREATE TYPE has two different syntax, one for composite type and one for internal user-defined type (I'm not sure either is defined in the standard, though) and I think we could do something like that. Or 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. - 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? - 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. - 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 - 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? For now, that's it. I'm going to dig more later. Thanks, Hitoshi Harada
Re: [HACKERS] [PATCH] Add transforms feature
Peter, I've been playing with the new patch, and haven't been able to reproduce the load problem created by the original patch. So that seems fixed. I'm not comfortable with having all of the transform mappings in the main contrib/ directory though. Can we add a subdirectory called transforms containing all of these? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 6/17/13 5:31 PM, Alvaro Herrera wrote: This is a large patch. Do you intend to push the whole thing as a single commit, or split it? I thought about splitting it up, but I didn't find a reasonable way to do it. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 6/14/13 11:48 PM, Craig Ringer wrote: I wonder if that should be extended to install headers for hstore, ltree, and while we're at it, intarray as well? Sure, if someone wants to go through and check which headers are independently usable, and do the necessarily cleanups with necessary. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter Eisentraut wrote: A transform is an SQL object that supplies to functions for converting between data types and procedural languages. For example, a transform could arrange that hstore is converted to an appropriate hash or dictionary object in PL/Perl or PL/Python. Externally visible changes: This is a large patch. Do you intend to push the whole thing as a single commit, or split it? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 06/18/2013 04:58 AM, Peter Eisentraut wrote: On 6/14/13 11:48 PM, Craig Ringer wrote: I wonder if that should be extended to install headers for hstore, ltree, and while we're at it, intarray as well? Sure, if someone wants to go through and check which headers are independently usable, and do the necessarily cleanups with necessary. I can do that if there are no objections. It's only tangental to this work really, so I'll post a separate thread when I get on to it. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
Peter Eisentraut pete...@gmx.net a écrit : A transform is an SQL object that supplies to functions for converting between data types and procedural languages. For example, a transform could arrange that hstore is converted to an appropriate hash or dictionary object in PL/Perl or PL/Python. Nice ! Continued from 2013-01 commit fest. All known open issues have been fixed. You kept PGXS style makefile... -- Envoyé de mon téléphone excusez la brièveté. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 6/14/13 3:46 AM, Cédric Villemain wrote: You kept PGXS style makefile... I know, but that's a separate issue that hasn't been decided yet. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add transforms feature
On 06/14/2013 11:11 AM, Peter Eisentraut wrote: A transform is an SQL object that supplies to functions for converting between data types and procedural languages. For example, a transform could arrange that hstore is converted to an appropriate hash or dictionary object in PL/Perl or PL/Python. Externally visible changes: - new SQL commands CREATE TRANSFORM and DROP TRANSFORM - system catalog pg_transform - transform support in PL/Perl and PL/Python - PL/Perl and PL/Python install their header files for use by external types I wonder if that should be extended to install headers for hstore, ltree, and while we're at it, intarray as well? You handle getting access to the headers by using include path flags: PG_CPPFLAGS = -I$(top_srcdir)/src/pl/plpython $(python_includespec) -I$(top_srcdir)/contrib/hstore which is fine for in-tree builds but AFAIK means that pgxs cannot build these extensions. If the extension modules installed their headers that problem would go away; you'd just #include contrib/hstore/hstore.h Many modules already have useful non-static functions that work with raw types and aren't callable via the fmgr, as well as the macros and typdefs that make working with them easier. intarray in particular would be extremely useful to use from other extensions, but in the past I've landed up copying it and then adding the extra functions I needed because I couldn't access its header in a PGXS build. Peter's already answered my main worry on this, which is whether any platform would prevent us from doing the required runtime symbol lookup from shared libs loaded at the same level. I knew RTLD_GLOBAL allowed it for dlopen(...), but wasn't sure about other platforms. Thoughts? Should contrib modules be able to install headers and use each others' symbols? I've seen interest in this in the wild from people who want to use hstore in their own extensions and I've had uses for it myself with intarray. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
