Thread: 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.c 13 Sep 2004 20:08:59 -0000 1.51 --- plperl.c 29 Sep 2004 15:18:29 -0000 *************** *** 82,87 **** --- 82,89 ---- bool lanpltrusted; bool fn_retistuple; /* true, if function returns tuple */ bool fn_retisset; /* true, if function returns set */ + bool fn_retisarray; /* true, if function returns "true" array*/ + bool fn_retispseudo; /* true, if function returns pseudo type*/ Oid ret_oid; /* Oid of returning type */ FmgrInfo result_in_func; Oid result_typioparam; *************** *** 89,94 **** --- 91,97 ---- FmgrInfo arg_out_func[FUNC_MAX_ARGS]; Oid arg_typioparam[FUNC_MAX_ARGS]; bool arg_is_rowtype[FUNC_MAX_ARGS]; + bool arg_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; i<len; 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_type typeStruct; + + 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_type retTypeStruct; + + 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, PointerGetDatum(SvPV(perlret, PL_na)), ObjectIdGetDatum(prodesc->result_typioparam), Int32GetDatum(-1)); ! } SvREFCNT_dec(perlret); --- 1212,1234 ---- } else + { + SV* ret; + + if (prodesc->fn_retisarray) + { + if(SvTYPE(SvRV(perlret))!=SVt_PVAV) elog(ERROR, "plperl: this function must return reference to array"); + ret = plperl_convert_to_pg_array(perlret); + SvREFCNT_dec(perlret); + perlret = ret; + } /* perl string to Datum */ retval = FunctionCall3(&prodesc->result_in_func, PointerGetDatum(SvPV(perlret, PL_na)), ObjectIdGetDatum(prodesc->result_typioparam), Int32GetDatum(-1)); ! } } SvREFCNT_dec(perlret); *************** *** 1384,1395 **** } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* Disallow pseudotype result, except VOID or RECORD */ if (typeStruct->typtype == 'p') { if (procStruct->prorettype == VOIDOID || ! procStruct->prorettype == RECORDOID) ! /* okay */ ; else if (procStruct->prorettype == TRIGGEROID) { free(prodesc->proname); --- 1457,1471 ---- } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* Disallow pseudotype result, except VOID, RECORD, ANYELEMENT or ANYARRAY*/ if (typeStruct->typtype == 'p') { if (procStruct->prorettype == VOIDOID || ! procStruct->prorettype == RECORDOID || ! procStruct->prorettype == ANYARRAYOID || ! procStruct->prorettype == ANYELEMENTOID) ! /* okay */ ! prodesc->fn_retispseudo=true; else if (procStruct->prorettype == TRIGGEROID) { free(prodesc->proname); *************** *** 1421,1426 **** --- 1497,1509 ---- procStruct->prorettype; } + if (procStruct->prorettype != ANYARRAYOID) + if (typeStruct->typlen == -1 && typeStruct->typelem) /*true, if function returns "true" array*/ + prodesc->fn_retisarray = true; + else + prodesc->fn_retisarray = false; + else prodesc->fn_retisarray = true; + perm_fmgr_info(typeStruct->typinput, &(prodesc->result_in_func)); prodesc->result_typioparam = getTypeIOParam(typeTup); *************** *** 1448,1463 **** } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); - /* Disallow pseudotype argument */ if (typeStruct->typtype == 'p') ! { ! free(prodesc->proname); ! free(prodesc); ! ereport(ERROR, ! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), ! errmsg("plperl functions cannot take type %s", ! format_type_be(procStruct->proargtypes[i])))); ! } if (typeStruct->typtype == 'c') prodesc->arg_is_rowtype[i] = true; --- 1531,1540 ---- } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); if (typeStruct->typtype == 'p') ! prodesc->arg_is_p[i] = true; ! else ! prodesc->arg_is_p[i] = false; if (typeStruct->typtype == 'c') prodesc->arg_is_rowtype[i] = true;
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)
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 ;)
Alvaro Herrera <alvherre@dcc.uchile.cl> 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
Sergej Sergeev <sergej@commandprompt.com> 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
>Sergej Sergeev <sergej@commandprompt.com> 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 -0000 1.51 --- plperl.c 30 Sep 2004 16:07:25 -0000 *************** *** 82,87 **** --- 82,89 ---- bool lanpltrusted; bool fn_retistuple; /* true, if function returns tuple */ bool fn_retisset; /* true, if function returns set */ + bool fn_retisarray; /* true, if function returns "true" array*/ + bool fn_retispseudo; /* true, if function returns pseudo type*/ Oid ret_oid; /* Oid of returning type */ FmgrInfo result_in_func; Oid result_typioparam; *************** *** 89,94 **** --- 91,97 ---- FmgrInfo arg_out_func[FUNC_MAX_ARGS]; Oid arg_typioparam[FUNC_MAX_ARGS]; bool arg_is_rowtype[FUNC_MAX_ARGS]; + bool arg_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; i<len; 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_type typeStruct; + + 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_type retTypeStruct; + + 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, PointerGetDatum(SvPV(perlret, PL_na)), ObjectIdGetDatum(prodesc->result_typioparam), Int32GetDatum(-1)); ! } SvREFCNT_dec(perlret); --- 1212,1234 ---- } else + { + SV* ret; + + if (prodesc->fn_retisarray) + { + if(SvTYPE(SvRV(perlret))!=SVt_PVAV) elog(ERROR, "plperl: this function must return reference to array"); + ret = plperl_convert_to_pg_array(perlret); + SvREFCNT_dec(perlret); + perlret = ret; + } /* perl string to Datum */ retval = FunctionCall3(&prodesc->result_in_func, PointerGetDatum(SvPV(perlret, PL_na)), ObjectIdGetDatum(prodesc->result_typioparam), Int32GetDatum(-1)); ! } } SvREFCNT_dec(perlret); *************** *** 1384,1395 **** } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* Disallow pseudotype result, except VOID or RECORD */ if (typeStruct->typtype == 'p') { if (procStruct->prorettype == VOIDOID || ! procStruct->prorettype == RECORDOID) ! /* okay */ ; else if (procStruct->prorettype == TRIGGEROID) { free(prodesc->proname); --- 1457,1471 ---- } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* Disallow pseudotype result, except VOID, RECORD, ANYELEMENT or ANYARRAY */ if (typeStruct->typtype == 'p') { if (procStruct->prorettype == VOIDOID || ! procStruct->prorettype == RECORDOID || ! procStruct->prorettype == ANYARRAYOID || ! procStruct->prorettype == ANYELEMENTOID) ! /* okay */ ! prodesc->fn_retispseudo=true; else if (procStruct->prorettype == TRIGGEROID) { free(prodesc->proname); *************** *** 1421,1426 **** --- 1497,1509 ---- procStruct->prorettype; } + if (procStruct->prorettype != ANYARRAYOID) + if (typeStruct->typlen == -1 && typeStruct->typelem) /*true, if function returns "true" array*/ + prodesc->fn_retisarray = true; + else + prodesc->fn_retisarray = false; + else prodesc->fn_retisarray = true; + perm_fmgr_info(typeStruct->typinput, &(prodesc->result_in_func)); prodesc->result_typioparam = getTypeIOParam(typeTup); *************** *** 1448,1455 **** } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* Disallow pseudotype argument */ if (typeStruct->typtype == 'p') { free(prodesc->proname); free(prodesc); --- 1531,1543 ---- } typeStruct = (Form_pg_type) GETSTRUCT(typeTup); ! /* 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); *************** *** 1458,1463 **** --- 1546,1553 ---- errmsg("plperl functions cannot take type %s", format_type_be(procStruct->proargtypes[i])))); } + else + prodesc->arg_is_p[i] = false; if (typeStruct->typtype == 'c') prodesc->arg_is_rowtype[i] = true;
any comments on my last patch? -- g.gRay: PL/perl, PL/PHP ;)
Sergej Sergeev wrote: > any comments on my last patch? > Speaking for myself, I am not really thinking much about new plperl stuff until we get closer to branching the code, which as I understand the current processes would be at the time we release an RC - that seems a little way off yet. cheers andrew
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 <sergej@commandprompt.com> 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 -0000 1.51 > --- plperl.c 30 Sep 2004 16:07:25 -0000 > *************** > *** 82,87 **** > --- 82,89 ---- > bool lanpltrusted; > bool fn_retistuple; /* true, if function returns tuple */ > bool fn_retisset; /* true, if function returns set */ > + bool fn_retisarray; /* true, if function returns "true" array*/ > + bool fn_retispseudo; /* true, if function returns pseudo type*/ > Oid ret_oid; /* Oid of returning type */ > FmgrInfo result_in_func; > Oid result_typioparam; > *************** > *** 89,94 **** > --- 91,97 ---- > FmgrInfo arg_out_func[FUNC_MAX_ARGS]; > Oid arg_typioparam[FUNC_MAX_ARGS]; > bool arg_is_rowtype[FUNC_MAX_ARGS]; > + bool arg_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; i<len; 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_type typeStruct; > + > + 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_type retTypeStruct; > + > + 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, > PointerGetDatum(SvPV(perlret, PL_na)), > ObjectIdGetDatum(prodesc->result_typioparam), > Int32GetDatum(-1)); > ! > } > > SvREFCNT_dec(perlret); > --- 1212,1234 ---- > > } > else > + { > + SV* ret; > + > + if (prodesc->fn_retisarray) > + { > + if(SvTYPE(SvRV(perlret))!=SVt_PVAV) elog(ERROR, "plperl: this function must return reference to array"); > + ret = plperl_convert_to_pg_array(perlret); > + SvREFCNT_dec(perlret); > + perlret = ret; > + } > /* perl string to Datum */ > > retval = FunctionCall3(&prodesc->result_in_func, > PointerGetDatum(SvPV(perlret, PL_na)), > ObjectIdGetDatum(prodesc->result_typioparam), > Int32GetDatum(-1)); > ! } > } > > SvREFCNT_dec(perlret); > *************** > *** 1384,1395 **** > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype result, except VOID or RECORD */ > if (typeStruct->typtype == 'p') > { > if (procStruct->prorettype == VOIDOID || > ! procStruct->prorettype == RECORDOID) > ! /* okay */ ; > else if (procStruct->prorettype == TRIGGEROID) > { > free(prodesc->proname); > --- 1457,1471 ---- > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype result, except VOID, RECORD, ANYELEMENT or ANYARRAY */ > if (typeStruct->typtype == 'p') > { > if (procStruct->prorettype == VOIDOID || > ! procStruct->prorettype == RECORDOID || > ! procStruct->prorettype == ANYARRAYOID || > ! procStruct->prorettype == ANYELEMENTOID) > ! /* okay */ > ! prodesc->fn_retispseudo=true; > else if (procStruct->prorettype == TRIGGEROID) > { > free(prodesc->proname); > *************** > *** 1421,1426 **** > --- 1497,1509 ---- > procStruct->prorettype; > } > > + if (procStruct->prorettype != ANYARRAYOID) > + if (typeStruct->typlen == -1 && typeStruct->typelem) /*true, if function returns "true" array*/ > + prodesc->fn_retisarray = true; > + else > + prodesc->fn_retisarray = false; > + else prodesc->fn_retisarray = true; > + > perm_fmgr_info(typeStruct->typinput, &(prodesc->result_in_func)); > prodesc->result_typioparam = getTypeIOParam(typeTup); > > *************** > *** 1448,1455 **** > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype argument */ > if (typeStruct->typtype == 'p') > { > free(prodesc->proname); > free(prodesc); > --- 1531,1543 ---- > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* 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); > *************** > *** 1458,1463 **** > --- 1546,1553 ---- > errmsg("plperl functions cannot take type %s", > format_type_be(procStruct->proargtypes[i])))); > } > + else > + prodesc->arg_is_p[i] = false; > > if (typeStruct->typtype == 'c') > prodesc->arg_is_rowtype[i] = true; > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- 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
Bruce Momjian <pgman@candle.pha.pa.us> writes: > I assume this is an 8.0 fix. It looks more like a new feature to me ... regards, tom lane
Tom Lane wrote:
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
They were requested features that we did not get done before freeze. It would be great if we couldBruce Momjian <pgman@candle.pha.pa.us> writes:I assume this is an 8.0 fix.It looks more like a new feature to me ...
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 - jd@commandprompt.com - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL
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. -- Peter Eisentraut http://developer.postgresql.org/~petere/
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 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
This has been saved for the 8.1 release: http:/momjian.postgresql.org/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Sergej Sergeev wrote: > > >Sergej Sergeev <sergej@commandprompt.com> 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 -0000 1.51 > --- plperl.c 30 Sep 2004 16:07:25 -0000 > *************** > *** 82,87 **** > --- 82,89 ---- > bool lanpltrusted; > bool fn_retistuple; /* true, if function returns tuple */ > bool fn_retisset; /* true, if function returns set */ > + bool fn_retisarray; /* true, if function returns "true" array*/ > + bool fn_retispseudo; /* true, if function returns pseudo type*/ > Oid ret_oid; /* Oid of returning type */ > FmgrInfo result_in_func; > Oid result_typioparam; > *************** > *** 89,94 **** > --- 91,97 ---- > FmgrInfo arg_out_func[FUNC_MAX_ARGS]; > Oid arg_typioparam[FUNC_MAX_ARGS]; > bool arg_is_rowtype[FUNC_MAX_ARGS]; > + bool arg_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; i<len; 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_type typeStruct; > + > + 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_type retTypeStruct; > + > + 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, > PointerGetDatum(SvPV(perlret, PL_na)), > ObjectIdGetDatum(prodesc->result_typioparam), > Int32GetDatum(-1)); > ! > } > > SvREFCNT_dec(perlret); > --- 1212,1234 ---- > > } > else > + { > + SV* ret; > + > + if (prodesc->fn_retisarray) > + { > + if(SvTYPE(SvRV(perlret))!=SVt_PVAV) elog(ERROR, "plperl: this function must return reference to array"); > + ret = plperl_convert_to_pg_array(perlret); > + SvREFCNT_dec(perlret); > + perlret = ret; > + } > /* perl string to Datum */ > > retval = FunctionCall3(&prodesc->result_in_func, > PointerGetDatum(SvPV(perlret, PL_na)), > ObjectIdGetDatum(prodesc->result_typioparam), > Int32GetDatum(-1)); > ! } > } > > SvREFCNT_dec(perlret); > *************** > *** 1384,1395 **** > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype result, except VOID or RECORD */ > if (typeStruct->typtype == 'p') > { > if (procStruct->prorettype == VOIDOID || > ! procStruct->prorettype == RECORDOID) > ! /* okay */ ; > else if (procStruct->prorettype == TRIGGEROID) > { > free(prodesc->proname); > --- 1457,1471 ---- > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype result, except VOID, RECORD, ANYELEMENT or ANYARRAY */ > if (typeStruct->typtype == 'p') > { > if (procStruct->prorettype == VOIDOID || > ! procStruct->prorettype == RECORDOID || > ! procStruct->prorettype == ANYARRAYOID || > ! procStruct->prorettype == ANYELEMENTOID) > ! /* okay */ > ! prodesc->fn_retispseudo=true; > else if (procStruct->prorettype == TRIGGEROID) > { > free(prodesc->proname); > *************** > *** 1421,1426 **** > --- 1497,1509 ---- > procStruct->prorettype; > } > > + if (procStruct->prorettype != ANYARRAYOID) > + if (typeStruct->typlen == -1 && typeStruct->typelem) /*true, if function returns "true" array*/ > + prodesc->fn_retisarray = true; > + else > + prodesc->fn_retisarray = false; > + else prodesc->fn_retisarray = true; > + > perm_fmgr_info(typeStruct->typinput, &(prodesc->result_in_func)); > prodesc->result_typioparam = getTypeIOParam(typeTup); > > *************** > *** 1448,1455 **** > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* Disallow pseudotype argument */ > if (typeStruct->typtype == 'p') > { > free(prodesc->proname); > free(prodesc); > --- 1531,1543 ---- > } > typeStruct = (Form_pg_type) GETSTRUCT(typeTup); > > ! /* 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); > *************** > *** 1458,1463 **** > --- 1546,1553 ---- > errmsg("plperl functions cannot take type %s", > format_type_be(procStruct->proargtypes[i])))); > } > + else > + prodesc->arg_is_p[i] = false; > > if (typeStruct->typtype == 'c') > prodesc->arg_is_rowtype[i] = true; > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- 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
Bruce Momjian wrote: >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. > > > 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. cheers andrew
>> >> > > 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 majordomo@postgresql.org) -- Command Prompt, Inc., home of Mammoth PostgreSQL - S/ODBC and S/JDBC Postgresql support, programming shared hosting and dedicated hosting. +1-503-667-4564 - jd@commandprompt.com - http://www.commandprompt.com PostgreSQL Replicator -- production quality replication for PostgreSQL
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 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
Sergej Sergeev wrote: > > >Sergej Sergeev <sergej@commandprompt.com> 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
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
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
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 >> >> >> > > >
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
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
Andrew Dunstan wrote: > > > 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 Added to TODO: o Pass arrays natively instead of as text between plperl and postgres o Add support for polymorphic arguments and return types to plperl -- 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
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.
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