plperl and inline functions -- first draft

Started by Joshua Tolleyabout 16 years ago32 messages
#1Joshua Tolley
eggyknap@gmail.com
1 attachment(s)

I've been trying to make pl/perl support 8.5's inline functions, with the
attached patch. The basics seem to be there, with at least one notable
exception, namely that plperl functions can do stuff only plperlu should do. I
presume this is because I really don't understand yet how plperl's trusted
interpreter initialization works, and have simply copied what looked like
important stuff from the original plperl call handler. I tested with this to
prove it:

DO $$ qx{touch test.txt}; $$ language plperl;

This works both with plperl and plperlu. Hints, anyone? Comments?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl_inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..33ede1b 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,870 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
--- 863,872 ----
  
  
  /*
!  * plperl_call_handler and plperl_inline_handler are the only
!  * externally-visible parts of the plperl call interface.  The Postgres function
!  * and trigger managers call plperl_call_handler to execute a perl function, and
!  * call plperl_inline_handler to execute plperl code in a DO statement.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 897,952 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+     FunctionCallInfoData fake_fcinfo;
+     FmgrInfo flinfo;
+     plperl_proc_desc desc;
+     HeapTuple	langTup;
+     Form_pg_language langStruct;
+ 
+     MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));                                                            
+     MemSet(&flinfo, 0, sizeof(flinfo));                                                                      
+     MemSet(&desc, 0, sizeof(desc));
+     fake_fcinfo.flinfo = &flinfo;                                                                            
+     flinfo.fn_oid = InvalidOid;                                                                              
+     flinfo.fn_mcxt = CurrentMemoryContext; 
+ 
+     desc.proname = "";
+     desc.fn_readonly = 0;
+ 
+     /************************************************************
+     * Lookup the pg_language tuple by Oid
+     ************************************************************/
+     langTup = SearchSysCache(LANGOID,
+                                 ObjectIdGetDatum(codeblock->langOid),
+                                 0, 0, 0);
+     if (!HeapTupleIsValid(langTup))
+     {
+         elog(ERROR, "cache lookup failed for language with OID %d",
+                 codeblock->langOid);
+     }
+     langStruct = (Form_pg_language) GETSTRUCT(langTup);
+     desc.lanpltrusted = langStruct->lanpltrusted;
+     ReleaseSysCache(langTup);
+     check_interp(desc.lanpltrusted);
+ 
+     desc.fn_retistuple = 0;
+     desc.fn_retisset = 0;
+     desc.fn_retisarray = 0;
+     desc.result_oid = VOIDOID;
+     desc.nargs = 0;
+     desc.reference = plperl_create_sub("", codeblock->source_text, 0);
+ 
+     plperl_call_perl_func(&desc, &fake_fcinfo);
+     PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
#2Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Tolley (#1)
Re: plperl and inline functions -- first draft

Joshua Tolley wrote:

I've been trying to make pl/perl support 8.5's inline functions, with the
attached patch.

Wow, this is the second time this week that people have produced patches
for stuff I was about to do. Cool!

The basics seem to be there, with at least one notable
exception, namely that plperl functions can do stuff only plperlu should do. I
presume this is because I really don't understand yet how plperl's trusted
interpreter initialization works, and have simply copied what looked like
important stuff from the original plperl call handler.

I'll check that out.

cheers

andrew

#3Joshua Tolley
eggyknap@gmail.com
In reply to: Andrew Dunstan (#2)
Re: plperl and inline functions -- first draft

On Thu, Nov 05, 2009 at 05:51:45PM -0500, Andrew Dunstan wrote:

Joshua Tolley wrote:

I've been trying to make pl/perl support 8.5's inline functions, with the
attached patch.

Wow, this is the second time this week that people have produced patches
for stuff I was about to do. Cool!

Well, I warmed up with PL/LOLCODE :)

The basics seem to be there, with at least one notable
exception, namely that plperl functions can do stuff only plperlu should do. I
presume this is because I really don't understand yet how plperl's trusted
interpreter initialization works, and have simply copied what looked like
important stuff from the original plperl call handler.

I'll check that out.

Many thanks.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Tolley (#3)
Re: plperl and inline functions -- first draft

Joshua Tolley wrote:

The basics seem to be there, with at least one notable
exception, namely that plperl functions can do stuff only plperlu should do. I
presume this is because I really don't understand yet how plperl's trusted
interpreter initialization works, and have simply copied what looked like
important stuff from the original plperl call handler.

I'll check that out.

Ok, I have a handle on the trusted/nontrusted issue. But I think the
piece that's missing here is that it needs to save the calling context
etc. and use PG_TRY() and friends, just like plperl_call_handler(). I'll
work on that.

cheers

andrew

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#4)
1 attachment(s)
Re: plperl and inline functions -- first draft

I wrote:

Ok, I have a handle on the trusted/nontrusted issue. But I think the
piece that's missing here is that it needs to save the calling context
etc. and use PG_TRY() and friends, just like plperl_call_handler().
I'll work on that.

OK, I committed the previously discussed change to store the language
trusted flag in the InlineCodeBlock structure. Following that, here is
my reworking of Josh's patch for DO blocks for plperl.

Missing are docs and regression tests.

cheers

andrew

Attachments:

plperl-inline.patchtext/x-patch; charset=iso-8859-1; name=plperl-inline.patchDownload
Index: src/include/catalog/pg_pltemplate.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_pltemplate.h,v
retrieving revision 1.8
diff -c -r1.8 pg_pltemplate.h
*** src/include/catalog/pg_pltemplate.h	22 Sep 2009 23:43:41 -0000	1.8
--- src/include/catalog/pg_pltemplate.h	6 Nov 2009 23:28:37 -0000
***************
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
Index: src/pl/plperl/plperl.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plperl/plperl.c,v
retrieving revision 1.153
diff -c -r1.153 plperl.c
*** src/pl/plperl/plperl.c	31 Oct 2009 18:11:59 -0000	1.153
--- src/pl/plperl/plperl.c	6 Nov 2009 23:28:37 -0000
***************
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
***************
*** 862,870 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
--- 863,872 ----
  
  
  /*
!  * plperl_call_handler and plperl_inline_handler are the only
!  * externally-visible parts of the plperl call interface.  The Postgres function
!  * and trigger managers call plperl_call_handler to execute a perl function, and
!  * call plperl_inline_handler to execute plperl code in a DO statement.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
***************
*** 895,900 ****
--- 897,960 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+     FunctionCallInfoData fake_fcinfo;
+     FmgrInfo flinfo;
+     plperl_proc_desc desc;
+     HeapTuple	langTup;
+     Form_pg_language langStruct;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+     MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));                                                            
+     MemSet(&flinfo, 0, sizeof(flinfo));                                                                      
+     MemSet(&desc, 0, sizeof(desc));
+     fake_fcinfo.flinfo = &flinfo;                                                                            
+     flinfo.fn_oid = InvalidOid;                                                                              
+     flinfo.fn_mcxt = CurrentMemoryContext; 
+ 
+     desc.proname = "Do Inline Block";
+     desc.fn_readonly = false;
+ 
+     desc.lanpltrusted = codeblock->langIsTrusted;
+ 
+     check_interp(desc.lanpltrusted);
+ 
+ 
+     desc.fn_retistuple = false;
+     desc.fn_retisset = false;
+     desc.fn_retisarray = false;
+     desc.result_oid = VOIDOID;
+     desc.nargs = 0;
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block", 
+ 									   codeblock->source_text, 
+ 									   desc.lanpltrusted);
+ 
+ 		plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+     PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#5)
Re: plperl and inline functions -- first draft

Andrew Dunstan <andrew@dunslane.net> writes:

! * plperl_call_handler and plperl_inline_handler are the only
! * externally-visible parts of the plperl call interface. The Postgres function
! * and trigger managers call plperl_call_handler to execute a perl function, and
! * call plperl_inline_handler to execute plperl code in a DO statement.

This comment should be updated to mention the validator. (What it
replaces was wrong before, but that's no excuse for not making it
right while you're touching it.)

The spacing seems a bit random too. pgindent will fix some of that,
but it's not very bright about making vertical spacing (ie extra
blank lines) consistent.

regards, tom lane

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#6)
Re: plperl and inline functions -- first draft

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

! * plperl_call_handler and plperl_inline_handler are the only
! * externally-visible parts of the plperl call interface. The Postgres function
! * and trigger managers call plperl_call_handler to execute a perl function, and
! * call plperl_inline_handler to execute plperl code in a DO statement.

This comment should be updated to mention the validator. (What it
replaces was wrong before, but that's no excuse for not making it
right while you're touching it.)

The spacing seems a bit random too. pgindent will fix some of that,
but it's not very bright about making vertical spacing (ie extra
blank lines) consistent.

OK, I'll clean it up.

cheers

andrew

#8Joshua Tolley
eggyknap@gmail.com
In reply to: Andrew Dunstan (#5)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Fri, Nov 06, 2009 at 06:37:38PM -0500, Andrew Dunstan wrote:

I wrote:

Ok, I have a handle on the trusted/nontrusted issue. But I think the
piece that's missing here is that it needs to save the calling context
etc. and use PG_TRY() and friends, just like plperl_call_handler().
I'll work on that.

OK, I committed the previously discussed change to store the language
trusted flag in the InlineCodeBlock structure. Following that, here is
my reworking of Josh's patch for DO blocks for plperl.

Missing are docs and regression tests.

Attached is a cleaned up comment with documentation. I looked through the
regression tests and didn't find any that used plperl -- should we add one for
this (or for this and all kinds of other stuff)? Is there some way to make
running the regression test conditional on having built --with-perl in the
first place?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl-inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..d4b2816 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE FUNCTION <replaceable>funcname</r
*** 59,64 ****
--- 59,75 ----
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
+ 
+    PL/Perl also supports inline functions called with the 
+    <xref linkend="sql-do" endterm="sql-do-title">
+    statement:
+ 
+ <programlisting>
+ DO $$
+     # PL/Perl function body
+ $$ LANGUAGE plperl;
+ </programlisting>
+ 
     The body of the function is ordinary Perl code. In fact, the PL/Perl
     glue code wraps it inside a Perl subroutine. A PL/Perl function must
     always return a scalar value.  You can return more complex structures
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..90b27fc 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,871 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
--- 863,877 ----
  
  
  /*
!  * There are three externally visible pieces to plperl: plperl_call_handler,
!  * plperl_inline_handler, and plperl_validator. The first gets called to run
!  * typical functions stored in pg_proc and created with CREATE FUNCTION as
!  * schema objects. The second handles one-time, "inline" functions called with
!  * the DO statement. Finally, the third validates a newly-created function at
!  * the time of the CREATE FUNCTION call. The precise behavior of the validator
!  * function may be modified by the check_function_bodies GUC.
   */
+ 
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 901,964 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+     FunctionCallInfoData fake_fcinfo;
+     FmgrInfo flinfo;
+     plperl_proc_desc desc;
+     HeapTuple	langTup;
+     Form_pg_language langStruct;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+     MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));                                                            
+     MemSet(&flinfo, 0, sizeof(flinfo));                                                                      
+     MemSet(&desc, 0, sizeof(desc));
+     fake_fcinfo.flinfo = &flinfo;                                                                            
+     flinfo.fn_oid = InvalidOid;                                                                              
+     flinfo.fn_mcxt = CurrentMemoryContext; 
+ 
+     desc.proname = "Do Inline Block";
+     desc.fn_readonly = false;
+ 
+     desc.lanpltrusted = codeblock->langIsTrusted;
+ 
+     check_interp(desc.lanpltrusted);
+ 
+ 
+     desc.fn_retistuple = false;
+     desc.fn_retisset = false;
+     desc.fn_retisarray = false;
+     desc.result_oid = VOIDOID;
+     desc.nargs = 0;
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block", 
+ 									   codeblock->source_text, 
+ 									   desc.lanpltrusted);
+ 
+ 		plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+     PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
#9Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Tolley (#8)
Re: plperl and inline functions -- first draft

Joshua Tolley wrote:

I looked through the
regression tests and didn't find any that used plperl -- should we add one for
this (or for this and all kinds of other stuff)? Is there some way to make
running the regression test conditional on having built --with-perl in the
first place?

Look in src/pl/plperl/{sql,expected}

cheers

andrew

#10Joshua Tolley
eggyknap@gmail.com
In reply to: Andrew Dunstan (#9)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Fri, Nov 06, 2009 at 09:53:20PM -0500, Andrew Dunstan wrote:

Joshua Tolley wrote:

I looked through the
regression tests and didn't find any that used plperl -- should we add one for
this (or for this and all kinds of other stuff)? Is there some way to make
running the regression test conditional on having built --with-perl in the
first place?

Look in src/pl/plperl/{sql,expected}

Ok, updated patch attached. As far as I know, this completes all outstanding
issues:

1) weird comment in plperl.c is corrected and formatted decently
2) plperlu vs. plperl actually works (thanks again, Andrew)
3) docs included
4) regression tests included

Some items of note include that this makes the regression tests add not only
plperl to the test database but also plperlu, which is a new thing. I can't
see why this might cause problems, but thought I'd mention it. The tests
specifically try to verify that plperl doesn't allow 'use Data::Dumper', and
plperlu does. Since Data::Dumper is part of perl core, that seemed safe, but
it is another dependency, and perhaps we don't want to do that. If not, is
there some other useful way of testing plperlu vs. plperl, and does it really
matter?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl-inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..d4b2816 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE FUNCTION <replaceable>funcname</r
*** 59,64 ****
--- 59,75 ----
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
+ 
+    PL/Perl also supports inline functions called with the 
+    <xref linkend="sql-do" endterm="sql-do-title">
+    statement:
+ 
+ <programlisting>
+ DO $$
+     # PL/Perl function body
+ $$ LANGUAGE plperl;
+ </programlisting>
+ 
     The body of the function is ordinary Perl code. In fact, the PL/Perl
     glue code wraps it inside a Perl subroutine. A PL/Perl function must
     always return a scalar value.  You can return more complex structures
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*************** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 ****
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 ----
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...3706018 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***************
*** 0 ****
--- 1,10 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ DO $$ elog(NOTICE, "This is plperlu"); $$ LANGUAGE plperlu;
+ NOTICE:  This is plperlu
+ DO $$ use Data::Dumper; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
+ DO $$ use Data::Dumper; $$ LANGUAGE plperlu;
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..3ff8441 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,871 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
--- 863,877 ----
  
  
  /*
!  * There are three externally visible pieces to plperl: plperl_call_handler,
!  * plperl_inline_handler, and plperl_validator. The first gets called to run
!  * typical functions stored in pg_proc and created with CREATE FUNCTION as
!  * schema objects. The second handles one-time, "inline" functions called with
!  * the DO statement. Finally, the third validates a newly-created function at
!  * the time of the CREATE FUNCTION call. The precise behavior of the validator
!  * function may be modified by the check_function_bodies GUC.
   */
+ 
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 901,962 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+     FunctionCallInfoData fake_fcinfo;
+     FmgrInfo flinfo;
+     plperl_proc_desc desc;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+     MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));                                                            
+     MemSet(&flinfo, 0, sizeof(flinfo));                                                                      
+     MemSet(&desc, 0, sizeof(desc));
+     fake_fcinfo.flinfo = &flinfo;                                                                            
+     flinfo.fn_oid = InvalidOid;                                                                              
+     flinfo.fn_mcxt = CurrentMemoryContext; 
+ 
+     desc.proname = "Do Inline Block";
+     desc.fn_readonly = false;
+ 
+     desc.lanpltrusted = codeblock->langIsTrusted;
+ 
+     check_interp(desc.lanpltrusted);
+ 
+ 
+     desc.fn_retistuple = false;
+     desc.fn_retisset = false;
+     desc.fn_retisarray = false;
+     desc.result_oid = VOIDOID;
+     desc.nargs = 0;
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block", 
+ 									   codeblock->source_text, 
+ 									   desc.lanpltrusted);
+ 
+ 		plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+     PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
diff --git a/src/pl/plperl/sql/plperl_do.sql b/src/pl/plperl/sql/plperl_do.sql
index ...4d02f97 .
*** a/src/pl/plperl/sql/plperl_do.sql
--- b/src/pl/plperl/sql/plperl_do.sql
***************
*** 0 ****
--- 1,10 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ 
+ DO $$ elog(NOTICE, "This is plperlu"); $$ LANGUAGE plperlu;
+ 
+ DO $$ use Data::Dumper; $$ LANGUAGE plperl;
+ 
+ DO $$ use Data::Dumper; $$ LANGUAGE plperlu;
#11Joshua Tolley
eggyknap@gmail.com
In reply to: Andrew Dunstan (#9)
Re: plperl and inline functions -- first draft

On Fri, Nov 06, 2009 at 09:53:20PM -0500, Andrew Dunstan wrote:

Joshua Tolley wrote:

I looked through the
regression tests and didn't find any that used plperl -- should we add one for
this (or for this and all kinds of other stuff)? Is there some way to make
running the regression test conditional on having built --with-perl in the
first place?

Look in src/pl/plperl/{sql,expected}

cheers

andrew

FWIW, I've added this to the upcoming commitfest page.

https://commitfest.postgresql.org/action/patch_view?id=206

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

#12Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Tolley (#10)
Re: plperl and inline functions -- first draft

Joshua Tolley wrote:

Some items of note include that this makes the regression tests add not only
plperl to the test database but also plperlu, which is a new thing. I can't
see why this might cause problems, but thought I'd mention it. The tests
specifically try to verify that plperl doesn't allow 'use Data::Dumper', and
plperlu does. Since Data::Dumper is part of perl core, that seemed safe, but
it is another dependency, and perhaps we don't want to do that. If not, is
there some other useful way of testing plperlu vs. plperl, and does it really
matter?

Loading both plperl and plperlu could have problems, as there are some
platforms where we can't use them both in the same session, due to some
perl oddities. We would need to test this on one such - I don't recall
which they are.

"Config" might be a better choice than "Data::Dumper". The Perl team or
some packagers could drop Data::Dumper some day, but they aren't likely
to drop Config.

cheers

andrew

#13Brendan Jurd
direvus@gmail.com
In reply to: Joshua Tolley (#10)
1 attachment(s)
Re: plperl and inline functions -- first draft

2009/11/10 Joshua Tolley <eggyknap@gmail.com>:

Ok, updated patch attached. As far as I know, this completes all outstanding
issues:

Hi Joshua,

I'm taking a look at this patch for the commitfest. I see that Andrew
has already taken an interest in the technical aspects of the patch,
so I'll focus on submission/code style/documentation.

I noticed that there was a fairly large amount of bogus/inconsistent
whitespace in the patch, particularly in the body of
plperl_inline_handler(). Some of the lines were indented with tabs,
others with spaces. You should stick with tabs. There were also a
lot of lines with a whole lot of trailing whitespace at the end.

See attached patch which repairs the whitespace. I see you generated
the patch with git, so I recommend `git diff --check`, it'll helpfully
report about some types of whitespace error.

In the documentation you refer to this feature as "inline functions".
I think this might be mixing up the terminology ... although the code
refers to "inline handlers" internally, the word "inline" doesn't
appear in the user-facing documentation for the DO command. Instead
they are referred to as "anonymous code blocks". I think it would
improve consistency if the PL/Perl mention used the same term.

Apart from those minor quibbles, the patch appears to apply, compile
and test fine, and work as advertised.

Cheers,
BJ

Attachments:

plperl-do-whitespace.patchapplication/octet-stream; name=plperl-do-whitespace.patchDownload
commit 657b5c164c9a20be57e29b1d6b7178a6448ef87f
Author: Brendan Jurd <direvus@gmail.com>
Date:   Sun Nov 15 11:51:28 2009 +1100

    Fix whitespace errors.

diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index d4b2816..cb4c7e4 100644
--- a/doc/src/sgml/plperl.sgml
+++ b/doc/src/sgml/plperl.sgml
@@ -60,7 +60,7 @@ CREATE FUNCTION <replaceable>funcname</replaceable> (<replaceable>argument-types
 $$ LANGUAGE plperl;
 </programlisting>
 
-   PL/Perl also supports inline functions called with the 
+   PL/Perl also supports inline functions called with the
    <xref linkend="sql-do" endterm="sql-do-title">
    statement:
 
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index d9eb4d3..fb94d33 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -906,38 +906,38 @@ PG_FUNCTION_INFO_V1(plperl_inline_handler);
 Datum
 plperl_inline_handler(PG_FUNCTION_ARGS)
 {
-	InlineCodeBlock *codeblock = (InlineCodeBlock *)
-	DatumGetPointer(PG_GETARG_DATUM(0)); FunctionCallInfoData fake_fcinfo;
-	FmgrInfo flinfo; plperl_proc_desc desc; plperl_call_data *save_call_data =
-	current_call_data; bool		oldcontext = trusted_context;
-
-    MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));                                                            
-    MemSet(&flinfo, 0, sizeof(flinfo));                                                                      
-    MemSet(&desc, 0, sizeof(desc));
-    fake_fcinfo.flinfo = &flinfo;                                                                            
-    flinfo.fn_oid = InvalidOid;                                                                              
-    flinfo.fn_mcxt = CurrentMemoryContext; 
+	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+	FunctionCallInfoData fake_fcinfo;
+	FmgrInfo flinfo;
+	plperl_proc_desc desc;
+	plperl_call_data *save_call_data = current_call_data;
+	bool oldcontext = trusted_context;
 
-    desc.proname = "Do Inline Block";
-    desc.fn_readonly = false;
+	MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));
+	MemSet(&flinfo, 0, sizeof(flinfo));
+	MemSet(&desc, 0, sizeof(desc));
+	fake_fcinfo.flinfo = &flinfo;
+	flinfo.fn_oid = InvalidOid;
+	flinfo.fn_mcxt = CurrentMemoryContext;
 
-    desc.lanpltrusted = codeblock->langIsTrusted;
+	desc.proname = "Do Inline Block";
+	desc.fn_readonly = false;
 
-    check_interp(desc.lanpltrusted);
+	desc.lanpltrusted = codeblock->langIsTrusted;
 
+	check_interp(desc.lanpltrusted);
 
-    desc.fn_retistuple = false;
-    desc.fn_retisset = false;
-    desc.fn_retisarray = false;
-    desc.result_oid = VOIDOID;
-    desc.nargs = 0;
+	desc.fn_retistuple = false;
+	desc.fn_retisset = false;
+	desc.fn_retisarray = false;
+	desc.result_oid = VOIDOID;
+	desc.nargs = 0;
 
 	PG_TRY();
 	{
-
-		desc.reference = plperl_create_sub("DO Inline Block", 
-									   codeblock->source_text, 
-									   desc.lanpltrusted);
+		desc.reference = plperl_create_sub("DO Inline Block",
+										   codeblock->source_text,
+										   desc.lanpltrusted);
 
 		plperl_call_perl_func(&desc, &fake_fcinfo);
 	}
@@ -952,7 +952,7 @@ plperl_inline_handler(PG_FUNCTION_ARGS)
 	current_call_data = save_call_data;
 	restore_context(oldcontext);
 
-    PG_RETURN_VOID();
+	PG_RETURN_VOID();
 }
 
 /*
#14Joshua Tolley
eggyknap@gmail.com
In reply to: Brendan Jurd (#13)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Sun, Nov 15, 2009 at 12:10:33PM +1100, Brendan Jurd wrote:

I noticed that there was a fairly large amount of bogus/inconsistent
whitespace in the patch, particularly in the body of
plperl_inline_handler(). Some of the lines were indented with tabs,
others with spaces. You should stick with tabs. There were also a
lot of lines with a whole lot of trailing whitespace at the end.

Thanks -- I tend to forget whitespace :)

In the documentation you refer to this feature as "inline functions".
I think this might be mixing up the terminology ... although the code
refers to "inline handlers" internally, the word "inline" doesn't
appear in the user-facing documentation for the DO command. Instead
they are referred to as "anonymous code blocks". I think it would
improve consistency if the PL/Perl mention used the same term.

I can accept that argument. The attached patch modifies the documentation, and
fixes another inconsistency I found.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl-inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE FUNCTION <replaceable>funcname</r
*** 59,69 ****
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. A PL/Perl function must
!    always return a scalar value.  You can return more complex structures
!    (arrays, records, and sets) by returning a reference, as discussed below.
!    Never return a list.
    </para>
  
    <note>
--- 59,81 ----
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
+ 
+    PL/Perl also supports anonymous code blocks called with the
+    <xref linkend="sql-do" endterm="sql-do-title">
+    statement:
+ 
+ <programlisting>
+ DO $$
+     # PL/Perl function body
+ $$ LANGUAGE plperl;
+ </programlisting>
+ 
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!    return a value; PL/Perl functions created with CREATE FUNCTION must always
!    return a scalar value. You can return more complex structures (arrays,
!    records, and sets) by returning a reference, as discussed below.  Never
!    return a list.
    </para>
  
    <note>
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*************** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 ****
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 ----
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...a955581 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***************
*** 0 ****
--- 1,7 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ DO $$ use Config; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..9120912 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,871 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
--- 863,877 ----
  
  
  /*
!  * There are three externally visible pieces to plperl: plperl_call_handler,
!  * plperl_inline_handler, and plperl_validator. The first gets called to run
!  * typical functions stored in pg_proc and created with CREATE FUNCTION as
!  * schema objects. The second handles one-time, "inline" functions called with
!  * the DO statement. Finally, the third validates a newly-created function at
!  * the time of the CREATE FUNCTION call. The precise behavior of the validator
!  * function may be modified by the check_function_bodies GUC.
   */
+ 
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 901,962 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+ 	FunctionCallInfoData fake_fcinfo;
+ 	FmgrInfo flinfo;
+ 	plperl_proc_desc desc;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+ 	MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));
+ 	MemSet(&flinfo, 0, sizeof(flinfo));
+ 	MemSet(&desc, 0, sizeof(desc));
+ 	fake_fcinfo.flinfo = &flinfo;
+ 	flinfo.fn_oid = InvalidOid;
+ 	flinfo.fn_mcxt = CurrentMemoryContext;
+ 
+ 	desc.proname = "Do Inline Block";
+ 	desc.fn_readonly = false;
+ 
+ 	desc.lanpltrusted = codeblock->langIsTrusted;
+ 
+ 	check_interp(desc.lanpltrusted);
+ 
+ 
+ 	desc.fn_retistuple = false;
+ 	desc.fn_retisset = false;
+ 	desc.fn_retisarray = false;
+ 	desc.result_oid = VOIDOID;
+ 	desc.nargs = 0;
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block",
+ 									   codeblock->source_text,
+ 									   desc.lanpltrusted);
+ 
+ 		plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+ 	PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
*************** plperl_call_perl_trigger_func(plperl_pro
*** 1171,1178 ****
  							  SV *td)
  {
  	dSP;
! 	SV		   *retval;
! 	Trigger    *tg_trigger;
  	int			i;
  	int			count;
  
--- 1233,1240 ----
  							  SV *td)
  {
  	dSP;
! 	SV			*retval;
! 	Trigger		*tg_trigger;
  	int			i;
  	int			count;
  
*************** plperl_func_handler(PG_FUNCTION_ARGS)
*** 1375,1381 ****
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 	
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
--- 1437,1443 ----
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
*************** hv_fetch_string(HV *hv, const char *key)
*** 2716,2724 ****
  }
  
  /*
!  * Provide function name for PL/Perl execution errors 
   */
! static void 
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
--- 2778,2786 ----
  }
  
  /*
!  * Provide function name for PL/Perl execution errors
   */
! static void
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
*************** plperl_exec_callback(void *arg)
*** 2727,2733 ****
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors 
   */
  static void
  plperl_compile_callback(void *arg)
--- 2789,2795 ----
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors
   */
  static void
  plperl_compile_callback(void *arg)
diff --git a/src/pl/plperl/sql/plperl_do.sql b/src/pl/plperl/sql/plperl_do.sql
index ...35745dd .
*** a/src/pl/plperl/sql/plperl_do.sql
--- b/src/pl/plperl/sql/plperl_do.sql
***************
*** 0 ****
--- 1,6 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ 
+ DO $$ use Config; $$ LANGUAGE plperl;
#15Brendan Jurd
direvus@gmail.com
In reply to: Joshua Tolley (#14)
Re: plperl and inline functions -- first draft

2009/11/17 Joshua Tolley <eggyknap@gmail.com>:

On Sun, Nov 15, 2009 at 12:10:33PM +1100, Brendan Jurd wrote:

I noticed that there was a fairly large amount of bogus/inconsistent
whitespace

...

Thanks -- I tend to forget whitespace :)

In the documentation you refer to this feature as "inline functions".
I think this might be mixing up the terminology

...

I can accept that argument. The attached patch modifies the documentation, and
fixes another inconsistency I found.

Cool. I have no gripes with the revised patch. I'm marking this as
ready for committer now. Thanks!

Cheers,
BJ

#16Joshua Tolley
eggyknap@gmail.com
In reply to: Brendan Jurd (#15)
Re: plperl and inline functions -- first draft

On Wed, Nov 18, 2009 at 09:35:35AM +1100, Brendan Jurd wrote:

2009/11/17 Joshua Tolley <eggyknap@gmail.com>:

On Sun, Nov 15, 2009 at 12:10:33PM +1100, Brendan Jurd wrote:

I noticed that there was a fairly large amount of bogus/inconsistent
whitespace

...

Thanks -- I tend to forget whitespace :)

In the documentation you refer to this feature as "inline functions".
I think this might be mixing up the terminology

...

I can accept that argument. The attached patch modifies the documentation, and
fixes another inconsistency I found.

Cool. I have no gripes with the revised patch. I'm marking this as
ready for committer now. Thanks!

Thanks to you, as well, and Andrew for his work.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

#17Alexey Klyukin
alexk@waki.ru
In reply to: Joshua Tolley (#10)
Re: plperl and inline functions -- first draft

On Nov 9, 2009, at 6:07 PM, Joshua Tolley wrote:

Ok, updated patch attached. As far as I know, this completes all outstanding
issues:

1) weird comment in plperl.c is corrected and formatted decently
2) plperlu vs. plperl actually works (thanks again, Andrew)
3) docs included
4) regression tests included

Some items of note include that this makes the regression tests add not only
plperl to the test database but also plperlu, which is a new thing. I can't
see why this might cause problems, but thought I'd mention it. The tests
specifically try to verify that plperl doesn't allow 'use Data::Dumper', and
plperlu does. Since Data::Dumper is part of perl core, that seemed safe, but
it is another dependency, and perhaps we don't want to do that. If not, is
there some other useful way of testing plperlu vs. plperl, and does it really
matter?

I've noticed that the patch doesn't install current_call_data before calling plperl_call_perl_func, although it saves and restores its previous value. This breaks spi code, which relies on current_call_data->prodesc, i.e.:

postgres=# DO $$ $result = spi_exec_query("select 1"); $$ LANGUAGE plperl;

server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

rogram received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00000001006f0336 in plperl_spi_exec (query=0x1007ecb60 "select 1", limit=0) at plperl.c:1895
warning: Source file is more recent than executable.
1895 spi_rv = SPI_execute(query, current_call_data->prodesc->fn_readonly,
(gdb) bt
#0 0x00000001006f0336 in plperl_spi_exec (query=0x1007ecb60 "select 1", limit=0) at plperl.c:1895

Also, a call to to plperl_call_perl_func should be cast to void to avoid a possible compiler warning (although It doesn't emit one on my system):

(void) plperl_call_perl_func(&desc, &fake_fcinfo);

--
Alexey Klyukin http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc

#18Andrew Dunstan
andrew@dunslane.net
In reply to: Alexey Klyukin (#17)
Re: plperl and inline functions -- first draft

Alexey Klyukin wrote:

I've noticed that the patch doesn't install current_call_data before calling plperl_call_perl_func, although it saves and restores its previous value. This breaks spi code, which relies on current_call_data->prodesc, i.e.:

postgres=# DO $$ $result = spi_exec_query("select 1"); $$ LANGUAGE plperl;

Yeah, good catch. We need to lift some stuff out of
plperl_func_handler(), because this code bypasses that. Not only setting
the call_data but also connectin g to the SPI manager and maybe one or
two other things.

Also, a call to to plperl_call_perl_func should be cast to void to avoid a possible compiler warning (although It doesn't emit one on my system):

(void) plperl_call_perl_func(&desc, &fake_fcinfo);

Right.

cheers

andrew

#19Joshua Tolley
eggyknap@gmail.com
In reply to: Andrew Dunstan (#18)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Tue, Nov 17, 2009 at 06:05:19PM -0500, Andrew Dunstan wrote:

Alexey Klyukin wrote:

I've noticed that the patch doesn't install current_call_data before calling plperl_call_perl_func, although it saves and restores its previous value. This breaks spi code, which relies on current_call_data->prodesc, i.e.:

postgres=# DO $$ $result = spi_exec_query("select 1"); $$ LANGUAGE plperl;

Yeah, good catch. We need to lift some stuff out of
plperl_func_handler(), because this code bypasses that. Not only setting
the call_data but also connectin g to the SPI manager and maybe one or
two other things.

I kept thinking I had to test SPI, but I guess I hadn't ever done it. The
attached takes care of such stuff, I think.

Also, a call to to plperl_call_perl_func should be cast to void to avoid a possible compiler warning (although It doesn't emit one on my system):

(void) plperl_call_perl_func(&desc, &fake_fcinfo);

Right.

I don't get the warning either, and didn't realize it could produce one.
Thanks -- that change is also in the attached version.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl-inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE FUNCTION <replaceable>funcname</r
*** 59,69 ****
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. A PL/Perl function must
!    always return a scalar value.  You can return more complex structures
!    (arrays, records, and sets) by returning a reference, as discussed below.
!    Never return a list.
    </para>
  
    <note>
--- 59,81 ----
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
+ 
+    PL/Perl also supports anonymous code blocks called with the
+    <xref linkend="sql-do" endterm="sql-do-title">
+    statement:
+ 
+ <programlisting>
+ DO $$
+     # PL/Perl function body
+ $$ LANGUAGE plperl;
+ </programlisting>
+ 
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!    return a value; PL/Perl functions created with CREATE FUNCTION must always
!    return a scalar value. You can return more complex structures (arrays,
!    records, and sets) by returning a reference, as discussed below.  Never
!    return a list.
    </para>
  
    <note>
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*************** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 ****
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 ----
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...a955581 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***************
*** 0 ****
--- 1,7 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ DO $$ use Config; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..9bb588b 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,871 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
--- 863,877 ----
  
  
  /*
!  * There are three externally visible pieces to plperl: plperl_call_handler,
!  * plperl_inline_handler, and plperl_validator. The first gets called to run
!  * typical functions stored in pg_proc and created with CREATE FUNCTION as
!  * schema objects. The second handles one-time, "inline" functions called with
!  * the DO statement. Finally, the third validates a newly-created function at
!  * the time of the CREATE FUNCTION call. The precise behavior of the validator
!  * function may be modified by the check_function_bodies GUC.
   */
+ 
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 901,970 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+ 	FunctionCallInfoData fake_fcinfo;
+ 	FmgrInfo flinfo;
+ 	plperl_proc_desc desc;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+ 	if (SPI_connect() != SPI_OK_CONNECT)
+ 		elog(ERROR, "could not connect to SPI manager");
+ 
+ 	MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));
+ 	MemSet(&flinfo, 0, sizeof(flinfo));
+ 	MemSet(&desc, 0, sizeof(desc));
+ 	fake_fcinfo.flinfo = &flinfo;
+ 	flinfo.fn_oid = InvalidOid;
+ 	flinfo.fn_mcxt = CurrentMemoryContext;
+ 
+ 	desc.proname = "Do Inline Block";
+ 	desc.fn_readonly = false;
+ 
+ 	desc.lanpltrusted = codeblock->langIsTrusted;
+ 	check_interp(desc.lanpltrusted);
+ 
+ 	desc.fn_retistuple = false;
+ 	desc.fn_retisset = false;
+ 	desc.fn_retisarray = false;
+ 	desc.result_oid = VOIDOID;
+ 	desc.nargs = 0;
+ 
+ 	current_call_data = (plperl_call_data *) palloc0(sizeof(plperl_call_data));
+ 	current_call_data->fcinfo = &fake_fcinfo;
+ 	current_call_data->prodesc = &desc;
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block",
+ 									   codeblock->source_text,
+ 									   desc.lanpltrusted);
+ 
+ 		(void) plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	if (SPI_finish() != SPI_OK_FINISH)
+ 		elog(ERROR, "SPI_finish() failed");
+ 
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+ 	PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
*************** plperl_call_perl_trigger_func(plperl_pro
*** 1171,1178 ****
  							  SV *td)
  {
  	dSP;
! 	SV		   *retval;
! 	Trigger    *tg_trigger;
  	int			i;
  	int			count;
  
--- 1241,1248 ----
  							  SV *td)
  {
  	dSP;
! 	SV			*retval;
! 	Trigger		*tg_trigger;
  	int			i;
  	int			count;
  
*************** plperl_func_handler(PG_FUNCTION_ARGS)
*** 1375,1381 ****
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 	
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
--- 1445,1451 ----
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
*************** hv_fetch_string(HV *hv, const char *key)
*** 2716,2724 ****
  }
  
  /*
!  * Provide function name for PL/Perl execution errors 
   */
! static void 
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
--- 2786,2794 ----
  }
  
  /*
!  * Provide function name for PL/Perl execution errors
   */
! static void
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
*************** plperl_exec_callback(void *arg)
*** 2727,2733 ****
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors 
   */
  static void
  plperl_compile_callback(void *arg)
--- 2797,2803 ----
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors
   */
  static void
  plperl_compile_callback(void *arg)
diff --git a/src/pl/plperl/sql/plperl_do.sql b/src/pl/plperl/sql/plperl_do.sql
index ...35745dd .
*** a/src/pl/plperl/sql/plperl_do.sql
--- b/src/pl/plperl/sql/plperl_do.sql
***************
*** 0 ****
--- 1,6 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ 
+ DO $$ use Config; $$ LANGUAGE plperl;
#20Andrew Dunstan
andrew@dunslane.net
In reply to: Joshua Tolley (#19)
Re: plperl and inline functions -- first draft

Joshua Tolley wrote:

+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 
+ 	if (SPI_connect() != SPI_OK_CONNECT)
+ 		elog(ERROR, "could not connect to SPI manager");

...

+ 	current_call_data = (plperl_call_data *) palloc0(sizeof(plperl_call_data));
+ 	current_call_data->fcinfo = &fake_fcinfo;
+ 	current_call_data->prodesc = &desc;	

I don't think this is done in the right order. If it is then this
comment in plperl_func_handler is wrong (as well as containing a typo):

/*
* Create the call_data beforing connecting to SPI, so that it is not
* allocated in the SPI memory context
*/

cheers

andrew

#21Alexey Klyukin
alexk@commandprompt.com
In reply to: Andrew Dunstan (#20)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Nov 18, 2009, at 5:46 AM, Andrew Dunstan wrote:

Joshua Tolley wrote:

+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ + 	if (SPI_connect() != SPI_OK_CONNECT)
+ 		elog(ERROR, "could not connect to SPI manager");

...

+ 	current_call_data = (plperl_call_data *) palloc0(sizeof(plperl_call_data));
+ 	current_call_data->fcinfo = &fake_fcinfo;
+ 	current_call_data->prodesc = &desc;	

I don't think this is done in the right order. If it is then this comment in plperl_func_handler is wrong (as well as containing a typo):

/*
* Create the call_data beforing connecting to SPI, so that it is not
* allocated in the SPI memory context
*/

Yes, current_call_data can't be allocate in the SPI memory context, since it's used to extract the result after SPI_finish is called, although it doesn't lead to problems here since no result is returned. Anyway, I'd move SPI_connect after the current_call_data initialization.

I also noticed that no error context is set in the inline handler, not sure whether it really useful except for the sake of consistency, but in case it is - here is the patch:

Attachments:

inline_callback.diffapplication/octet-stream; name=inline_callback.diff; x-unix-mode=0600Download
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index a955581..f5d56ed 100644
--- a/src/pl/plperl/expected/plperl_do.out
+++ b/src/pl/plperl/expected/plperl_do.out
@@ -3,5 +3,7 @@ DO $$
   elog(NOTICE, $a);
 $$ LANGUAGE plperl;
 NOTICE:  This is a test
+CONTEXT:  PL/Perl anonymous code block
 DO $$ use Config; $$ LANGUAGE plperl;
 ERROR:  'require' trapped by operation mask at line 1.
+CONTEXT:  PL/Perl anonymous code block
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 9bb588b..7f5d98e 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -165,6 +165,7 @@ static SV  *plperl_create_sub(char *proname, char *s, bool trusted);
 static SV  *plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo);
 static void plperl_compile_callback(void *arg);
 static void plperl_exec_callback(void *arg);
+static void plperl_inline_callback(void *arg);
 
 /*
  * This routine is a crock, and so is everyplace that calls it.  The problem
@@ -912,6 +913,13 @@ plperl_inline_handler(PG_FUNCTION_ARGS)
 	plperl_proc_desc desc;
 	plperl_call_data *save_call_data = current_call_data;
 	bool		oldcontext = trusted_context;
+	ErrorContextCallback pl_error_context;
+	
+	/* Set a callback for error reporting */
+	pl_error_context.callback = plperl_inline_callback;
+	pl_error_context.previous = error_context_stack;
+	pl_error_context.arg = (Datum) 0;
+	error_context_stack = &pl_error_context;
 
 	if (SPI_connect() != SPI_OK_CONNECT)
 		elog(ERROR, "could not connect to SPI manager");
@@ -959,6 +967,7 @@ plperl_inline_handler(PG_FUNCTION_ARGS)
 	if (SPI_finish() != SPI_OK_FINISH)
 		elog(ERROR, "SPI_finish() failed");
 
+	error_context_stack = pl_error_context.previous;
 	current_call_data = save_call_data;
 	restore_context(oldcontext);
 
@@ -2806,3 +2815,12 @@ plperl_compile_callback(void *arg)
 	if (procname)
 		errcontext("compilation of PL/Perl function \"%s\"", procname);
 }
+
+/*
+ * Error context for the inline handler
+ */
+static void
+plperl_inline_callback(void *arg)
+{
+	errcontext("PL/Perl anonymous code block");
+}
#22Joshua Tolley
eggyknap@gmail.com
In reply to: Alexey Klyukin (#21)
1 attachment(s)
Re: plperl and inline functions -- first draft

On Wed, Nov 18, 2009 at 12:38:00PM +0200, Alexey Klyukin wrote:

Yes, current_call_data can't be allocate in the SPI memory context, since it's used to extract the result after SPI_finish is called, although it doesn't lead to problems here since no result is returned. Anyway, I'd move SPI_connect after the current_call_data initialization.

I also noticed that no error context is set in the inline handler, not sure whether it really useful except for the sake of consistency, but in case it is - here is the patch:

Makes sense on both counts. Thanks for the help. How does the attached look?

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

Attachments:

plperl-inline.patchtext/x-diff; charset=us-asciiDownload
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 49631f2..ebcb608 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
*************** CREATE FUNCTION <replaceable>funcname</r
*** 59,69 ****
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. A PL/Perl function must
!    always return a scalar value.  You can return more complex structures
!    (arrays, records, and sets) by returning a reference, as discussed below.
!    Never return a list.
    </para>
  
    <note>
--- 59,81 ----
      # PL/Perl function body
  $$ LANGUAGE plperl;
  </programlisting>
+ 
+    PL/Perl also supports anonymous code blocks called with the
+    <xref linkend="sql-do" endterm="sql-do-title">
+    statement:
+ 
+ <programlisting>
+ DO $$
+     # PL/Perl function body
+ $$ LANGUAGE plperl;
+ </programlisting>
+ 
     The body of the function is ordinary Perl code. In fact, the PL/Perl
!    glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
!    return a value; PL/Perl functions created with CREATE FUNCTION must always
!    return a scalar value. You can return more complex structures (arrays,
!    records, and sets) by returning a reference, as discussed below.  Never
!    return a list.
    </para>
  
    <note>
diff --git a/src/include/catalog/pg_pltemplate.h b/src/include/catalog/pg_pltemplate.h
index 5ef97df..8cdedb4 100644
*** a/src/include/catalog/pg_pltemplate.h
--- b/src/include/catalog/pg_pltemplate.h
*************** typedef FormData_pg_pltemplate *Form_pg_
*** 70,77 ****
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" _null_ "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
--- 70,77 ----
  DATA(insert ( "plpgsql"		t t "plpgsql_call_handler" "plpgsql_inline_handler" "plpgsql_validator" "$libdir/plpgsql" _null_ ));
  DATA(insert ( "pltcl"		t t "pltcl_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
  DATA(insert ( "pltclu"		f f "pltclu_call_handler" _null_ _null_ "$libdir/pltcl" _null_ ));
! DATA(insert ( "plperl"		t t "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
! DATA(insert ( "plperlu"		f f "plperl_call_handler" "plperl_inline_handler" "plperl_validator" "$libdir/plperl" _null_ ));
  DATA(insert ( "plpythonu"	f f "plpython_call_handler" _null_ _null_ "$libdir/plpython" _null_ ));
  
  #endif   /* PG_PLTEMPLATE_H */
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..2c32850 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*************** OBJS = plperl.o spi_internal.o SPI.o
*** 38,45 ****
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
--- 38,45 ----
  
  SHLIB_LINK = $(perl_embed_ldflags)
  
! REGRESS_OPTS = --dbname=$(PL_TESTDB) --load-language=plperl --load-language=plperlu
! REGRESS = plperl plperl_trigger plperl_shared plperl_elog plperl_do
  # where to find psql for running the tests
  PSQLDIR = $(bindir)
  
diff --git a/src/pl/plperl/expected/plperl_do.out b/src/pl/plperl/expected/plperl_do.out
index ...86337f3 .
*** a/src/pl/plperl/expected/plperl_do.out
--- b/src/pl/plperl/expected/plperl_do.out
***************
*** 0 ****
--- 1,9 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ NOTICE:  This is a test
+ CONTEXT: PL/Perl anonymous code block
+ DO $$ use Config; $$ LANGUAGE plperl;
+ ERROR:  'require' trapped by operation mask at line 1.
+ CONTEXT: PL/Perl anonymous code block
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 4ed4f59..88b73f3 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static plperl_call_data *current_call_da
*** 144,149 ****
--- 144,150 ----
   * Forward declarations
   **********************************************************************/
  Datum		plperl_call_handler(PG_FUNCTION_ARGS);
+ Datum		plperl_inline_handler(PG_FUNCTION_ARGS);
  Datum		plperl_validator(PG_FUNCTION_ARGS);
  void		_PG_init(void);
  
*************** static SV  *plperl_create_sub(char *pron
*** 164,169 ****
--- 165,171 ----
  static SV  *plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo);
  static void plperl_compile_callback(void *arg);
  static void plperl_exec_callback(void *arg);
+ static void plperl_inline_callback(void *arg);
  
  /*
   * This routine is a crock, and so is everyplace that calls it.  The problem
*************** plperl_modify_tuple(HV *hvTD, TriggerDat
*** 862,871 ****
  
  
  /*
!  * This is the only externally-visible part of the plperl call interface.
!  * The Postgres function and trigger managers call it to execute a
!  * perl function.
   */
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
--- 864,878 ----
  
  
  /*
!  * There are three externally visible pieces to plperl: plperl_call_handler,
!  * plperl_inline_handler, and plperl_validator. The first gets called to run
!  * typical functions stored in pg_proc and created with CREATE FUNCTION as
!  * schema objects. The second handles one-time, "inline" functions called with
!  * the DO statement. Finally, the third validates a newly-created function at
!  * the time of the CREATE FUNCTION call. The precise behavior of the validator
!  * function may be modified by the check_function_bodies GUC.
   */
+ 
  PG_FUNCTION_INFO_V1(plperl_call_handler);
  
  Datum
*************** plperl_call_handler(PG_FUNCTION_ARGS)
*** 895,900 ****
--- 902,980 ----
  	return retval;
  }
  
+ PG_FUNCTION_INFO_V1(plperl_inline_handler);
+ 
+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {
+ 	InlineCodeBlock *codeblock = (InlineCodeBlock *) DatumGetPointer(PG_GETARG_DATUM(0));
+ 	FunctionCallInfoData fake_fcinfo;
+ 	FmgrInfo flinfo;
+ 	plperl_proc_desc desc;
+ 	plperl_call_data *save_call_data = current_call_data;
+ 	bool		oldcontext = trusted_context;
+ 	ErrorContextCallback pl_error_context;
+ 
+ 	/* Set a callback for error reporting */
+ 	pl_error_context.callback = plperl_inline_callback;
+ 	pl_error_context.previous = error_context_stack;
+ 	pl_error_context.arg = (Datum) 0;
+ 	error_context_stack = &pl_error_context;
+ 
+ 	MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));
+ 	MemSet(&flinfo, 0, sizeof(flinfo));
+ 	MemSet(&desc, 0, sizeof(desc));
+ 	fake_fcinfo.flinfo = &flinfo;
+ 	flinfo.fn_oid = InvalidOid;
+ 	flinfo.fn_mcxt = CurrentMemoryContext;
+ 
+ 	desc.proname = "Do Inline Block";
+ 	desc.fn_readonly = false;
+ 
+ 	desc.lanpltrusted = codeblock->langIsTrusted;
+ 	check_interp(desc.lanpltrusted);
+ 
+ 	desc.fn_retistuple = false;
+ 	desc.fn_retisset = false;
+ 	desc.fn_retisarray = false;
+ 	desc.result_oid = VOIDOID;
+ 	desc.nargs = 0;
+ 
+ 	current_call_data = (plperl_call_data *) palloc0(sizeof(plperl_call_data));
+ 	current_call_data->fcinfo = &fake_fcinfo;
+ 	current_call_data->prodesc = &desc;
+ 
+ 	if (SPI_connect() != SPI_OK_CONNECT)
+ 		elog(ERROR, "could not connect to SPI manager");
+ 
+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block",
+ 									   codeblock->source_text,
+ 									   desc.lanpltrusted);
+ 
+ 		(void) plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		error_context_stack = pl_error_context.previous;
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	if (SPI_finish() != SPI_OK_FINISH)
+ 		elog(ERROR, "SPI_finish() failed");
+ 
+ 	error_context_stack = pl_error_context.previous;
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+ 	PG_RETURN_VOID();
+ }
+ 
  /*
   * This is the other externally visible function - it is called when CREATE
   * FUNCTION is issued to validate the function being created/replaced.
*************** plperl_call_perl_trigger_func(plperl_pro
*** 1171,1178 ****
  							  SV *td)
  {
  	dSP;
! 	SV		   *retval;
! 	Trigger    *tg_trigger;
  	int			i;
  	int			count;
  
--- 1251,1258 ----
  							  SV *td)
  {
  	dSP;
! 	SV			*retval;
! 	Trigger		*tg_trigger;
  	int			i;
  	int			count;
  
*************** plperl_func_handler(PG_FUNCTION_ARGS)
*** 1375,1381 ****
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 	
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
--- 1455,1461 ----
  
  	/* Restore the previous error callback */
  	error_context_stack = pl_error_context.previous;
! 
  	if (array_ret == NULL)
  		SvREFCNT_dec(perlret);
  
*************** hv_fetch_string(HV *hv, const char *key)
*** 2716,2724 ****
  }
  
  /*
!  * Provide function name for PL/Perl execution errors 
   */
! static void 
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
--- 2796,2804 ----
  }
  
  /*
!  * Provide function name for PL/Perl execution errors
   */
! static void
  plperl_exec_callback(void *arg)
  {
  	char *procname = (char *) arg;
*************** plperl_exec_callback(void *arg)
*** 2727,2733 ****
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors 
   */
  static void
  plperl_compile_callback(void *arg)
--- 2807,2813 ----
  }
  
  /*
!  * Provide function name for PL/Perl compilation errors
   */
  static void
  plperl_compile_callback(void *arg)
*************** plperl_compile_callback(void *arg)
*** 2736,2738 ****
--- 2816,2827 ----
  	if (procname)
  		errcontext("compilation of PL/Perl function \"%s\"", procname);
  }
+ 
+ /*
+  * Error context for the inline handler
+  */
+ static void
+ plperl_inline_callback(void *arg)
+ {
+ 	errcontext("PL/Perl anonymous code block");
+ }
diff --git a/src/pl/plperl/sql/plperl_do.sql b/src/pl/plperl/sql/plperl_do.sql
index ...35745dd .
*** a/src/pl/plperl/sql/plperl_do.sql
--- b/src/pl/plperl/sql/plperl_do.sql
***************
*** 0 ****
--- 1,6 ----
+ DO $$
+   $a = 'This is a test';
+   elog(NOTICE, $a);
+ $$ LANGUAGE plperl;
+ 
+ DO $$ use Config; $$ LANGUAGE plperl;
#23u235sentinel
u235sentinel@gmail.com
In reply to: Joshua Tolley (#22)
plruby code and postgres ?

Does anyone have a link for pl/ruby? I found a link under the postgres
documentation and found a web site from there talking about the code.
However when I clicked on the link to download it I noticed ftp wouldn't
respond on their site.

Thanks!

#24Tim Bunce
Tim.Bunce@pobox.com
In reply to: Joshua Tolley (#22)
Re: plperl and inline functions -- first draft

On Thu, Nov 19, 2009 at 05:04:22PM -0700, Joshua Tolley wrote:

The body of the function is ordinary Perl code. In fact, the PL/Perl
! glue code wraps it inside a Perl subroutine. Anonymous code blocks cannot
! return a value; PL/Perl functions created with CREATE FUNCTION must always
! return a scalar value. You can return more complex structures (arrays,
! records, and sets) by returning a reference, as discussed below. Never
! return a list.
</para>

The "must always" and "Never return a list" seem needlessly strong, not
very helpful, and slightly misleading. The key point is that the call is
made in a scalar context. The implications of that follow naturally.

I'd suggest:

...; PL/Perl functions created with CREATE FUNCTION are called in a
scalar context, so can't return a list. You can return more complex
structures (arrays, records, and sets) by returning a reference, as
discussed below.

That only mentions "functions created with CREATE FUNCTION" though.
Perhaps it needs to be generalized to cover DO as well.

+ Datum
+ plperl_inline_handler(PG_FUNCTION_ARGS)
+ {

+ desc.proname = "Do Inline Block";

+ 	PG_TRY();
+ 	{
+ 
+ 		desc.reference = plperl_create_sub("DO Inline Block",
+ 									   codeblock->source_text,
+ 									   desc.lanpltrusted);
+ 
+ 		(void) plperl_call_perl_func(&desc, &fake_fcinfo);
+ 	}
+ 	PG_CATCH();
+ 	{
+ 		error_context_stack = pl_error_context.previous;
+ 		current_call_data = save_call_data;
+ 		restore_context(oldcontext);
+ 		PG_RE_THROW();
+ 	}
+ 	PG_END_TRY();
+ 
+ 	if (SPI_finish() != SPI_OK_FINISH)
+ 		elog(ERROR, "SPI_finish() failed");
+ 
+ 	error_context_stack = pl_error_context.previous;
+ 	current_call_data = save_call_data;
+ 	restore_context(oldcontext);
+ 
+ 	PG_RETURN_VOID();

When does the reference held by desc.reference get freed?
At the moment it looks like this would leak memory for each DO.

+ static void
+ plperl_inline_callback(void *arg)
+ {
+ 	errcontext("PL/Perl anonymous code block");
+ }

I'd like to see more consistent terminlogy:

desc.proname = "Do Inline Block";
plperl_create_sub("DO Inline Block",
errcontext("PL/Perl anonymous code block");

Tim.

#25Alexey Klyukin
alexk@commandprompt.com
In reply to: Joshua Tolley (#22)
Re: plperl and inline functions -- first draft

On Nov 20, 2009, at 2:04 AM, Joshua Tolley wrote:

On Wed, Nov 18, 2009 at 12:38:00PM +0200, Alexey Klyukin wrote:

Yes, current_call_data can't be allocate in the SPI memory context, since it's used to extract the result after SPI_finish is called, although it doesn't lead to problems here since no result is returned. Anyway, I'd move SPI_connect after the current_call_data initialization.

I also noticed that no error context is set in the inline handler, not sure whether it really useful except for the sake of consistency, but in case it is - here is the patch:

Makes sense on both counts. Thanks for the help. How does the attached look?

These two problems seem to be fixed now, thank you.

--
Alexey Klyukin http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc

#26Alexey Klyukin
alexk@commandprompt.com
In reply to: Tim Bunce (#24)
Re: plperl and inline functions -- first draft

On Nov 20, 2009, at 3:50 PM, Tim Bunce wrote:

When does the reference held by desc.reference get freed?
At the moment it looks like this would leak memory for each DO.

Isn't it also the case with the existing plperl code ? I've noticed that free(prodesc) is called when it's no longer used (i.e. in plperl_compile_callback:1636), but refcount of desc->reference is never decremented.

--
Alexey Klyukin http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc

#27Ross J. Reedstrom
reedstrm@rice.edu
In reply to: u235sentinel (#23)
Re: plruby code and postgres ?

On Thu, Nov 19, 2009 at 05:15:05PM -0700, u235sentinel wrote:

Does anyone have a link for pl/ruby? I found a link under the postgres
documentation and found a web site from there talking about the code.
However when I clicked on the link to download it I noticed ftp wouldn't
respond on their site.

Debian's got a copy of the original tarball or the most recently release
version:

http://packages.debian.org/source/lenny/postgresql-plruby

And links there in.

Ross
--
Ross Reedstrom, Ph.D. reedstrm@rice.edu
Systems Engineer & Admin, Research Scientist phone: 713-348-6166
The Connexions Project http://cnx.org fax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE

#28David E. Wheeler
david@kineticode.com
In reply to: Tim Bunce (#24)
Re: plperl and inline functions -- first draft

On Nov 20, 2009, at 10:50 PM, Tim Bunce wrote:

I'd suggest:

...; PL/Perl functions created with CREATE FUNCTION are called in a
scalar context, so can't return a list. You can return more complex
structures (arrays, records, and sets) by returning a reference, as
discussed below.

That only mentions "functions created with CREATE FUNCTION" though.
Perhaps it needs to be generalized to cover DO as well.

FWIW, DO is run in a VOID context. Return values are ignored (or perhaps trigger an exception?).

Best,

David

#29Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alexey Klyukin (#26)
Re: plperl and inline functions -- first draft

Alexey Klyukin <alexk@commandprompt.com> writes:

On Nov 20, 2009, at 3:50 PM, Tim Bunce wrote:

When does the reference held by desc.reference get freed?
At the moment it looks like this would leak memory for each DO.

Isn't it also the case with the existing plperl code ? I've noticed that free(prodesc) is called when it's no longer used (i.e. in plperl_compile_callback:1636), but refcount of desc->reference is never decremented.

I've been experimenting with this and confirmed that there is a leak;
not only in the DO patch but in the pre-existing code, if a plperl
function is redefined repeatedly.

Is this the correct way to release the SV* reference?

if (reference)
SvREFCNT_dec(reference);

regards, tom lane

#30Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joshua Tolley (#22)
Re: plperl and inline functions -- first draft

Joshua Tolley <eggyknap@gmail.com> writes:

Makes sense on both counts. Thanks for the help. How does the attached look?

Applied with minor corrections, mainly around the state save/restore
logic. I also put in some code to fix the memory leak noted by Tim Bunce,
but am waiting for some confirmation that it's right before
back-patching the pre-existing bug of the same ilk.

regards, tom lane

#31Alexey Klyukin
alexk@commandprompt.com
In reply to: Tom Lane (#29)
Re: plperl and inline functions -- first draft

On Nov 29, 2009, at 4:40 AM, Tom Lane wrote:

Alexey Klyukin <alexk@commandprompt.com> writes:

Isn't it also the case with the existing plperl code ? I've noticed that free(prodesc) is called when it's no longer used (i.e. in plperl_compile_callback:1636), but refcount of desc->reference is never decremented.

I've been experimenting with this and confirmed that there is a leak;
not only in the DO patch but in the pre-existing code, if a plperl
function is redefined repeatedly.

Is this the correct way to release the SV* reference?

if (reference)
SvREFCNT_dec(reference);

Yes. In fact this only decreases the reference count, making the interpreter free the memory referred to when it becomes 0, but since prodesc->reference has refcount of 1 this would do the right thing.

--
Alexey Klyukin http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc

#32Joshua Tolley
eggyknap@gmail.com
In reply to: Tom Lane (#30)
Re: plperl and inline functions -- first draft

On Sat, Nov 28, 2009 at 10:15:40PM -0500, Tom Lane wrote:

Joshua Tolley <eggyknap@gmail.com> writes:

Makes sense on both counts. Thanks for the help. How does the attached look?

Applied with minor corrections, mainly around the state save/restore
logic. I also put in some code to fix the memory leak noted by Tim Bunce,
but am waiting for some confirmation that it's right before
back-patching the pre-existing bug of the same ilk.

regards, tom lane

Yay, and thanks. For the record, I'm can't claim to know whether your fix is
the Right Thing or not, so I'm witholding comment.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com