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)

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

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 ;)

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 ;)

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:

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 ...

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 - 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