Re: [PATCHES] plperl features
Bruce Momjian wrote: Sergej, are you going to repost this patch? Sorry for delaying. Yes, I working on it, but I wait for decision about Andrew and Abhijit patches. ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] plperl features
Sergej Sergeev said: Bruce Momjian wrote: Sergej, are you going to repost this patch? Sorry for delaying. Yes, I working on it, but I wait for decision about Andrew and Abhijit patches. This is the polymorphic types plus perl to pg array patch, right? I am not working on this right now (or anything else for 8.1) - the original plperl array to pg array implementation was broken and I was not happy about the fix I first came up with, and ran out of time to work on an acceptable solution. Neither is Abhijit, to the best of my knowledge - he has today submitted a patch for SPI fetching via cursor, which should not affect this stuff. We recently put the two items on the TODO list, as I understood from Joshua that you guys weren't working on plperl at all in the 8.1 feature freeze time frame. cheers andrew ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] plperl features
Do we need a TODO item? --- Andrew Dunstan wrote: This was the patch that I took the array processing piece from and attempted to fix, since it was badly broken. However, I'm not happy about any of the ways of doing it, and suspect I won't get it done for 8.1. I think we need that piece done before we look at ANYELEMENT/ANYARRAY. cheers andrew Bruce Momjian wrote: Sergej, are you going to repost this patch? --- Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: Also, I don't think the arg_is_p variable is really the proper fix for this, but I am unsure what to recomment. Others? The thing I didn't like about that was that it assumes there is only one pseudotype behavior that is or ever will be interesting for plperl. I think it'd probably make more sense to store an array of the parameter type OIDs and then check for ANYELEMENT or ANYARRAY as such in the places where the patch uses arg_is_p. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] plperl features
Bruce Momjian wrote: Do we need a TODO item? Sure, Maybe two: . pass arrays natively instead of as text between plperl and postgres . add support for polymorphic arguments and return types to plperl cheers andrew ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] plperl features
Sergej Sergeev wrote: Sergej Sergeev [EMAIL PROTECTED] writes: What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Other pseudo-types are disallowed (no-change) No, because you diked out the check at lines 1452ff, rather than upgrading it to something correct. I find the fn_retispseudo and arg_is_p flags pretty bogus anyway since they fail to indicate *which* pseudotype it is. You might as well just test for the specific type OID. regards, tom lane New patch. I have added the check pseudo-type argumetns. Specific type is substituted in runtime, during function call. I can't apply this patch because the code has changed too much. Would you regenerate a patch against current CVS? Also, this indenting seems wrong: ! /* Disallow pseudotype argument, except ANYELEMENT or ANYARRAY */ if (typeStruct-typtype == 'p') + if (procStruct-proargtypes[i] == ANYARRAYOID || + procStruct-proargtypes[i] == ANYELEMENTOID) + /* okay */ + prodesc-arg_is_p[i] = true; + else { free(prodesc-proname); free(prodesc); Putting an 'if' after an 'if' is just too strange. Please make a more complete fix that has proper block indenting. Also, I don't think the arg_is_p variable is really the proper fix for this, but I am unsure what to recomment. Others? -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] plperl features
Bruce Momjian pgman@candle.pha.pa.us writes: Also, I don't think the arg_is_p variable is really the proper fix for this, but I am unsure what to recomment. Others? The thing I didn't like about that was that it assumes there is only one pseudotype behavior that is or ever will be interesting for plperl. I think it'd probably make more sense to store an array of the parameter type OIDs and then check for ANYELEMENT or ANYARRAY as such in the places where the patch uses arg_is_p. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] plperl features
I assume this is an 8.0 fix. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. --- Sergej Sergeev wrote: Sergej Sergeev [EMAIL PROTECTED] writes: What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Other pseudo-types are disallowed (no-change) No, because you diked out the check at lines 1452ff, rather than upgrading it to something correct. I find the fn_retispseudo and arg_is_p flags pretty bogus anyway since they fail to indicate *which* pseudotype it is. You might as well just test for the specific type OID. regards, tom lane New patch. I have added the check pseudo-type argumetns. Specific type is substituted in runtime, during function call. -- g.gRay: PL/perl, PL/PHP ;) Index: plperl.c === RCS file: /projects/cvsroot/pgsql-server/src/pl/plperl/plperl.c,v retrieving revision 1.51 diff -c -w -r1.51 plperl.c *** plperl.c 13 Sep 2004 20:08:59 - 1.51 --- plperl.c 30 Sep 2004 16:07:25 - *** *** 82,87 --- 82,89 boollanpltrusted; boolfn_retistuple; /* true, if function returns tuple */ boolfn_retisset;/* true, if function returns set */ + boolfn_retisarray; /* true, if function returns true array*/ + boolfn_retispseudo; /* true, if function returns pseudo type*/ Oid ret_oid;/* Oid of returning type */ FmgrInforesult_in_func; Oid result_typioparam; *** *** 89,94 --- 91,97 FmgrInfoarg_out_func[FUNC_MAX_ARGS]; Oid arg_typioparam[FUNC_MAX_ARGS]; boolarg_is_rowtype[FUNC_MAX_ARGS]; + boolarg_is_p[FUNC_MAX_ARGS]; SV *reference; } plperl_proc_desc; *** *** 277,282 --- 280,319 } /** + * convert perl array to the string representation + **/ + static SV* + plperl_convert_to_pg_array(SV *src) + { + SV* rv; + SV**val;; + AV* internal; + int len, + i; + + internal=(AV*)SvRV(src); + len = av_len(internal)+1; + + rv = newSVpv({ ,0); + for(i=0; ilen; i++) + { + val = av_fetch(internal, i, FALSE); + if (SvTYPE(*val)==SVt_RV) + if (SvTYPE(SvRV(*val))==SVt_PVAV) + sv_catpvf(rv, %s, SvPV(plperl_convert_to_pg_array(*val),PL_na)); + else + elog(ERROR, plperl: check array structure); + else + sv_catpvf(rv, %s, SvPV(*val,PL_na)); + if (i != len-1) sv_catpvf(rv, ,); + } + + sv_catpvf(rv, }); + + return rv; + } + + /** * turn a tuple into a hash expression and add it to a list **/ static void *** *** 752,757 --- 789,807 XPUSHs(sv_2mortal(newSVpv(undef, 0))); for (i = 0; i desc-nargs; i++) { + if (desc-arg_is_p[i]){ + HeapTuple typeTup; + Form_pg_typetypeStruct; + + typeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_argtype(fcinfo-flinfo, i)), + 0, 0, 0); + typeStruct = (Form_pg_type) GETSTRUCT(typeTup); + perm_fmgr_info(typeStruct-typoutput, (desc-arg_out_func[i])); + desc-arg_typioparam[i] = getTypeIOParam(typeTup); + ReleaseSysCache(typeTup); + } + if (desc-arg_is_rowtype[i]) { if (fcinfo-argnull[i]) *** *** 909,914 --- 959,977 perlret = srf_perlret; } + if (prodesc-fn_retispseudo){ + HeapTuple retTypeTup; + Form_pg_typeretTypeStruct; + + retTypeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_rettype(fcinfo-flinfo)), + 0, 0,
Re: [PATCHES] plperl features
Bruce Momjian [EMAIL PROTECTED] writes: I assume this is an 8.0 fix. It looks more like a new feature to me ... regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] plperl features
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I assume this is an 8.0 fix. It looks more like a new feature to me ... They were requested features that we did not get done before freeze. It would be great if we could get them applied. We are continuing to develop plPerl and at this point, what is going to ship with 8.0 won't even be close to what is available via the plPerl website. We expected this to a degree of course, but if we can get some of them in, it would be nice for the community who wants to use plPerl. Sincerely, Joshua D. Drake regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-667-4564 - [EMAIL PROTECTED] - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL
Re: [PATCHES] plperl features
Peter Eisentraut wrote: Joshua D. Drake wrote: We expected this to a degree of course, but if we can get some of them in, it would be nice for the community who wants to use plPerl. On the other hand, it wouldn't be that nice for the community that respects a good freeze. As you know, there will always be one more feature. Considering that we're already a long time into the beta phase, and we're still working out portability issues especially in the various plug-ins, we really ought to be strict about the freeze in that area if we ever want to get finished. Agreed. This is the downside of being bundled with the server. Sorry. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] plperl features
Entirely expected (at least by me). I certainly respect a good freeze (what a nice phrase). However, there are outstanding patches from Abhijit Menon-Sen that are genuine bug fixes that need to be queued, reviewed and applied. It was worth a shot ;) Joshua D. Drake cheers andrew ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED]) -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-667-4564 - [EMAIL PROTECTED] - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] plperl features
Andrew Dunstan wrote: Considering that we're already a long time into the beta phase, and we're still working out portability issues especially in the various plug-ins, we really ought to be strict about the freeze in that area if we ever want to get finished. Agreed. This is the downside of being bundled with the server. Sorry. Entirely expected (at least by me). I certainly respect a good freeze (what a nice phrase). However, there are outstanding patches from Abhijit Menon-Sen that are genuine bug fixes that need to be queued, reviewed and applied. Right, I have not gotten to them yet. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plperl features
Sergej Sergeev [EMAIL PROTECTED] writes: What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Other pseudo-types are disallowed (no-change) No, because you diked out the check at lines 1452ff, rather than upgrading it to something correct. I find the fn_retispseudo and arg_is_p flags pretty bogus anyway since they fail to indicate *which* pseudotype it is. You might as well just test for the specific type OID. regards, tom lane New patch. I have added the check pseudo-type argumetns. Specific type is substituted in runtime, during function call. -- g.gRay: PL/perl, PL/PHP ;) Index: plperl.c === RCS file: /projects/cvsroot/pgsql-server/src/pl/plperl/plperl.c,v retrieving revision 1.51 diff -c -w -r1.51 plperl.c *** plperl.c13 Sep 2004 20:08:59 - 1.51 --- plperl.c30 Sep 2004 16:07:25 - *** *** 82,87 --- 82,89 boollanpltrusted; boolfn_retistuple; /* true, if function returns tuple */ boolfn_retisset;/* true, if function returns set */ + boolfn_retisarray; /* true, if function returns true array*/ + boolfn_retispseudo; /* true, if function returns pseudo type*/ Oid ret_oid;/* Oid of returning type */ FmgrInforesult_in_func; Oid result_typioparam; *** *** 89,94 --- 91,97 FmgrInfoarg_out_func[FUNC_MAX_ARGS]; Oid arg_typioparam[FUNC_MAX_ARGS]; boolarg_is_rowtype[FUNC_MAX_ARGS]; + boolarg_is_p[FUNC_MAX_ARGS]; SV *reference; } plperl_proc_desc; *** *** 277,282 --- 280,319 } /** + * convert perl array to the string representation + **/ + static SV* + plperl_convert_to_pg_array(SV *src) + { + SV* rv; + SV**val;; + AV* internal; + int len, + i; + + internal=(AV*)SvRV(src); + len = av_len(internal)+1; + + rv = newSVpv({ ,0); + for(i=0; ilen; i++) + { + val = av_fetch(internal, i, FALSE); + if (SvTYPE(*val)==SVt_RV) + if (SvTYPE(SvRV(*val))==SVt_PVAV) + sv_catpvf(rv, %s, SvPV(plperl_convert_to_pg_array(*val),PL_na)); + else + elog(ERROR, plperl: check array structure); + else + sv_catpvf(rv, %s, SvPV(*val,PL_na)); + if (i != len-1) sv_catpvf(rv, ,); + } + + sv_catpvf(rv, }); + + return rv; + } + + /** * turn a tuple into a hash expression and add it to a list **/ static void *** *** 752,757 --- 789,807 XPUSHs(sv_2mortal(newSVpv(undef, 0))); for (i = 0; i desc-nargs; i++) { + if (desc-arg_is_p[i]){ + HeapTuple typeTup; + Form_pg_typetypeStruct; + + typeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_argtype(fcinfo-flinfo, i)), + 0, 0, 0); + typeStruct = (Form_pg_type) GETSTRUCT(typeTup); + perm_fmgr_info(typeStruct-typoutput, (desc-arg_out_func[i])); + desc-arg_typioparam[i] = getTypeIOParam(typeTup); + ReleaseSysCache(typeTup); + } + if (desc-arg_is_rowtype[i]) { if (fcinfo-argnull[i]) *** *** 909,914 --- 959,977 perlret = srf_perlret; } + if (prodesc-fn_retispseudo){ + HeapTuple retTypeTup; + Form_pg_typeretTypeStruct; + + retTypeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_rettype(fcinfo-flinfo)), + 0, 0, 0); + retTypeStruct = (Form_pg_type) GETSTRUCT(retTypeTup); + perm_fmgr_info(retTypeStruct-typinput, (prodesc-result_in_func)); + prodesc-result_typioparam = getTypeIOParam(retTypeTup); + ReleaseSysCache(retTypeTup); + } + if (prodesc-fn_retisset SRF_IS_FIRSTCALL()) { if (prodesc-fn_retistuple)
[PATCHES] plperl features
Patch provide support for array type and pseudo type (anyelement, anyarray) for function parameters and result. for example: CREATE FUNCTION add_three_values(anyelement, anyelement, anyelement) RETURNS anyelement AS ' return $_[0]+$_[1]+$_[2]; ' LANGUAGE plperl; CREATE FUNCTION make_array(anyelement, anyelement, anyelement) RETURNS anyarray AS ' return [$_[0], $_[1], $_[2]]; ' LANGUAGE plperl; Comments? --ggRay Index: plperl.c === RCS file: /projects/cvsroot/pgsql-server/src/pl/plperl/plperl.c,v retrieving revision 1.51 diff -c -w -r1.51 plperl.c *** plperl.c13 Sep 2004 20:08:59 - 1.51 --- plperl.c29 Sep 2004 15:18:29 - *** *** 82,87 --- 82,89 boollanpltrusted; boolfn_retistuple; /* true, if function returns tuple */ boolfn_retisset;/* true, if function returns set */ + boolfn_retisarray; /* true, if function returns true array*/ + boolfn_retispseudo; /* true, if function returns pseudo type*/ Oid ret_oid;/* Oid of returning type */ FmgrInforesult_in_func; Oid result_typioparam; *** *** 89,94 --- 91,97 FmgrInfoarg_out_func[FUNC_MAX_ARGS]; Oid arg_typioparam[FUNC_MAX_ARGS]; boolarg_is_rowtype[FUNC_MAX_ARGS]; + boolarg_is_p[FUNC_MAX_ARGS]; SV *reference; } plperl_proc_desc; *** *** 277,282 --- 280,319 } /** + * convert perl array to the string representation + **/ + static SV* + plperl_convert_to_pg_array(SV *src) + { + SV* rv; + SV**val;; + AV* internal; + int len, + i; + + internal=(AV*)SvRV(src); + len = av_len(internal)+1; + + rv = newSVpv({ ,0); + for(i=0; ilen; i++) + { + val = av_fetch(internal, i, FALSE); + if (SvTYPE(*val)==SVt_RV) + if (SvTYPE(SvRV(*val))==SVt_PVAV) + sv_catpvf(rv, %s, SvPV(plperl_convert_to_pg_array(*val),PL_na)); + else + elog(ERROR, plperl: check array structure); + else + sv_catpvf(rv, %s, SvPV(*val,PL_na)); + if (i != len-1) sv_catpvf(rv, ,); + } + + sv_catpvf(rv, }); + + return rv; + } + + /** * turn a tuple into a hash expression and add it to a list **/ static void *** *** 752,757 --- 789,807 XPUSHs(sv_2mortal(newSVpv(undef, 0))); for (i = 0; i desc-nargs; i++) { + if (desc-arg_is_p[i]){ + HeapTuple typeTup; + Form_pg_typetypeStruct; + + typeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_argtype(fcinfo-flinfo, i)), + 0, 0, 0); + typeStruct = (Form_pg_type) GETSTRUCT(typeTup); + perm_fmgr_info(typeStruct-typoutput, (desc-arg_out_func[i])); + desc-arg_typioparam[i] = getTypeIOParam(typeTup); + ReleaseSysCache(typeTup); + } + if (desc-arg_is_rowtype[i]) { if (fcinfo-argnull[i]) *** *** 909,914 --- 959,977 perlret = srf_perlret; } + if (prodesc-fn_retispseudo){ + HeapTuple retTypeTup; + Form_pg_typeretTypeStruct; + + retTypeTup = SearchSysCache(TYPEOID, + ObjectIdGetDatum(get_fn_expr_rettype(fcinfo-flinfo)), + 0, 0, 0); + retTypeStruct = (Form_pg_type) GETSTRUCT(retTypeTup); + perm_fmgr_info(retTypeStruct-typinput, (prodesc-result_in_func)); + prodesc-result_typioparam = getTypeIOParam(retTypeTup); + ReleaseSysCache(retTypeTup); + } + if (prodesc-fn_retisset SRF_IS_FIRSTCALL()) { if (prodesc-fn_retistuple) *** *** 1149,1161 } else /* perl string to Datum */ retval = FunctionCall3(prodesc-result_in_func,
Re: [PATCHES] plperl features
On Wed, Sep 29, 2004 at 07:13:47PM +0300, Sergej Sergeev wrote: Patch provide support for array type and pseudo type (anyelement, anyarray) for function parameters and result. for example: CREATE FUNCTION add_three_values(anyelement, anyelement, anyelement) RETURNS anyelement AS ' return $_[0]+$_[1]+$_[2]; ' LANGUAGE plperl; CREATE FUNCTION make_array(anyelement, anyelement, anyelement) RETURNS anyarray AS ' return [$_[0], $_[1], $_[2]]; ' LANGUAGE plperl; Comments? What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) Java is clearly an example of a money oriented programming (A. Stepanov) ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] plperl features
Alvaro Herrera wrote: On Wed, Sep 29, 2004 at 07:13:47PM +0300, Sergej Sergeev wrote: Patch provide support for array type and pseudo type (anyelement, anyarray) for function parameters and result. for example: CREATE FUNCTION add_three_values(anyelement, anyelement, anyelement) RETURNS anyelement AS ' return $_[0]+$_[1]+$_[2]; ' LANGUAGE plperl; CREATE FUNCTION make_array(anyelement, anyelement, anyelement) RETURNS anyarray AS ' return [$_[0], $_[1], $_[2]]; ' LANGUAGE plperl; Comments? What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Other pseudo-types are disallowed (no-change) --- g.gRay: PL/perl, PL/PHP ;) ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] plperl features
Alvaro Herrera [EMAIL PROTECTED] writes: What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Indeed. This patch breaks those defenses... regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] plperl features
Sergej Sergeev [EMAIL PROTECTED] writes: What happens if you feed other pseudotypes, like cstring or language_handler? Shouldn't that be disallowed or something? Other pseudo-types are disallowed (no-change) No, because you diked out the check at lines 1452ff, rather than upgrading it to something correct. I find the fn_retispseudo and arg_is_p flags pretty bogus anyway since they fail to indicate *which* pseudotype it is. You might as well just test for the specific type OID. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster