Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
Andrew Dunstan wrote: Yes. I believe the test is highlighting an existing problem: that plperl function in non-PG_UTF8 databases can't use regular expressions that require unicode character meta-data. Either the (GetDatabaseEncoding() == PG_UTF8) test in plperl_safe_init() should be removed, so the utf8fix function is always called, or the test should be removed (or hacked to only apply to PG_UTF8 databases). I tried forcing the test, but it doesn't seem to work, possibly because in the case that the db is not utf8 we aren't forcing argument strings to UTF8 :-( I think we might need to remove the test from the patch. I have not been able to come up with a fix for this - the whole thing seems very fragile. I'm going to commit what remains of this patch, but not add the extra regression test. I'll add a TODO to allow plperl to do utf8 operations in non-utf8 databases. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
Tim Bunce wrote:
On Fri, Dec 25, 2009 at 12:54:13PM -0500, Andrew Dunstan wrote:
Tim Bunce wrote:
I've attached an update of my previous refactoring of plperl.c.
It's been rebased over the current (git) HEAD and has a few
very minor additions.
[snip]
+ -- Test compilation of unicode regex
+ --
+ CREATE OR REPLACE FUNCTION perl_unicode_regex(text) RETURNS INTEGER AS $$
+ # see http://rt.perl.org/rt3/Ticket/Display.html?id=47576
+ return ($_[0] =~ /\x{263A}|happy/i) ? 1 : 0; # unicode smiley
+ $$ LANGUAGE plperl;
This test is failing on my setup at least when the target db is not UTF8
encoded.
Maybe that's a bug we need to fix?
Yes. I believe the test is highlighting an existing problem: that plperl
function in non-PG_UTF8 databases can't use regular expressions that
require unicode character meta-data.
Either the (GetDatabaseEncoding() == PG_UTF8) test in plperl_safe_init()
should be removed, so the utf8fix function is always called, or the
test should be removed (or hacked to only apply to PG_UTF8 databases).
I tried forcing the test, but it doesn't seem to work, possibly because
in the case that the db is not utf8 we aren't forcing argument strings
to UTF8 :-(
I think we might need to remove the test from the patch.
p.s. There may be other problems using unicode in non-PG_UTF8 databases,
but I believe this patch doesn't change the behaviour for better or worse.
Right.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
On Fri, Dec 25, 2009 at 12:54:13PM -0500, Andrew Dunstan wrote:
> Tim Bunce wrote:
>> I've attached an update of my previous refactoring of plperl.c.
>> It's been rebased over the current (git) HEAD and has a few
>> very minor additions.
>>
> [snip]
>> + -- Test compilation of unicode regex
>> + --
>> + CREATE OR REPLACE FUNCTION perl_unicode_regex(text) RETURNS INTEGER AS $$
>> + # see http://rt.perl.org/rt3/Ticket/Display.html?id=47576
>> + return ($_[0] =~ /\x{263A}|happy/i) ? 1 : 0; # unicode smiley
>> + $$ LANGUAGE plperl;
>
> This test is failing on my setup at least when the target db is not UTF8
> encoded.
>
> Maybe that's a bug we need to fix?
Yes. I believe the test is highlighting an existing problem: that plperl
function in non-PG_UTF8 databases can't use regular expressions that
require unicode character meta-data.
Either the (GetDatabaseEncoding() == PG_UTF8) test in plperl_safe_init()
should be removed, so the utf8fix function is always called, or the
test should be removed (or hacked to only apply to PG_UTF8 databases).
Tim.
p.s. There may be other problems using unicode in non-PG_UTF8 databases,
but I believe this patch doesn't change the behaviour for better or worse.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
Tim Bunce wrote:
I've attached an update of my previous refactoring of plperl.c.
It's been rebased over the current (git) HEAD and has a few
very minor additions.
[snip]
+ -- Test compilation of unicode regex
+ --
+ CREATE OR REPLACE FUNCTION perl_unicode_regex(text) RETURNS INTEGER AS $$
+ # see http://rt.perl.org/rt3/Ticket/Display.html?id=47576
+ return ($_[0] =~ /\x{263A}|happy/i) ? 1 : 0; # unicode smiley
+ $$ LANGUAGE plperl;
This test is failing on my setup at least when the target db is not UTF8
encoded.
Maybe that's a bug we need to fix?
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
On Thu, Dec 03, 2009 at 03:47:21PM -0800, Josh Berkus wrote: > Tim, > > Since there's a commitfest on right now, meaningful feedback on your > patch could be delayed. Just so you know. Understood. Thanks Josh. Tim. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
Tim, Since there's a commitfest on right now, meaningful feedback on your patch could be delayed. Just so you know. --Josh Berkus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Initial refactoring of plperl.c - rebased [PATCH]
I've attached an update of my previous refactoring of plperl.c.
It's been rebased over the current (git) HEAD and has a few
very minor additions.
Background:
I've started work on the enhancements to plperl I outlined on pg-general
(in the "Wishlist of PL/Perl Enhancements for 8.5" thread).
I have a working implementation of those changes, plus some performance
enhancements, that I'm now re-working into a clean set of tested and
polished patches.
This patch is a first step that doesn't add any extra functionality.
It refactors the internals to make adding the extra functionality
easier (and more clearly visible).
Changes in this patch:
- Changed MULTIPLICITY check from runtime to compiletime.
No loads the large Config module.
- Changed plperl_init_interp() to return new interp
and not alter the global interp_state
- Moved plperl_safe_init() call into check_interp().
- Removed plperl_safe_init_done state variable
as interp_state now covers that role.
- Changed plperl_create_sub() to take a plperl_proc_desc argument.
- Simplified return value handling in plperl_create_sub.
- Added a test for the effect of the utf8fix function.
- Changed perl.com link in the docs to perl.org and tweaked
wording to clarify that require, not use, is what's blocked.
- Moved perl code in large multi-line C string literal macros
out to plc_*.pl files.
- Added a test2macro.pl utility to convert the plc_*.pl files to
macros in a perlchunks.h file which is #included
Additions since previous verion:
- Replaced calls to SvPV(val, PL_na) with SvPV_nolen(val)
- Simplifed plperl_safe_init() slightly
- Removed trailing whitespace from new plc_*.pl files.
I'd appreciate any feedback on the patch.
Tim.
diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml
index 7eebfba..37114bd 100644
*** a/doc/src/sgml/plperl.sgml
--- b/doc/src/sgml/plperl.sgml
***
*** 14,20
PL/Perl is a loadable procedural language that enables you to write
PostgreSQL functions in the
!http://www.perl.com";>Perl programming language.
--- 14,20
PL/Perl is a loadable procedural language that enables you to write
PostgreSQL functions in the
!http://www.perl.org";>Perl programming language.
*** SELECT * FROM perl_set();
*** 313,319
use strict;
in the function body. But this only works in PL/PerlU
!functions, since use is not a trusted operation. In
PL/Perl functions you can instead do:
BEGIN { strict->import(); }
--- 313,320
use strict;
in the function body. But this only works in PL/PerlU
!functions, since the use triggers a require
!which is not a trusted operation. In
PL/Perl functions you can instead do:
BEGIN { strict->import(); }
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index a3c3495..8989b14 100644
*** a/src/pl/plperl/GNUmakefile
--- b/src/pl/plperl/GNUmakefile
*** PSQLDIR = $(bindir)
*** 45,50
--- 45,55
include $(top_srcdir)/src/Makefile.shlib
+ plperl.o: perlchunks.h
+
+ perlchunks.h: plc_*.pl
+ $(PERL) text2macro.pl --strip='^(\#.*|\s*)$$' plc_*.pl > perlchunks.htmp
+ mv perlchunks.htmp perlchunks.h
all: all-lib
*** submake:
*** 65,71
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
clean distclean maintainer-clean: clean-lib
! rm -f SPI.c $(OBJS)
rm -rf results
rm -f regression.diffs regression.out
--- 70,76
$(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
clean distclean maintainer-clean: clean-lib
! rm -f SPI.c $(OBJS) perlchunks.htmp perlchunks.h
rm -rf results
rm -f regression.diffs regression.out
diff --git a/src/pl/plperl/expected/plperl.out b/src/pl/plperl/expected/plperl.out
index b942739..c1cf7ae 100644
*** a/src/pl/plperl/expected/plperl.out
--- b/src/pl/plperl/expected/plperl.out
*** CONTEXT: PL/Perl anonymous code block
*** 566,568
--- 566,575
DO $$ use Config; $$ LANGUAGE plperl;
ERROR: 'require' trapped by operation mask at line 1.
CONTEXT: PL/Perl anonymous code block
+ --
+ -- Test compilation of unicode regex
+ --
+ CREATE OR REPLACE FUNCTION perl_unicode_regex(text) RETURNS INTEGER AS $$
+ # see http://rt.perl.org/rt3/Ticket/Display.html?id=47576
+ return ($_[0] =~ /\x{263A}|happy/i) ? 1 : 0; # unicode smiley
+ $$ LANGUAGE plperl;
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index ...d2d5518 .
*** a/src/pl/plperl/plc_perlboot.pl
--- b/src/pl/plperl/plc_perlboot.pl
***
*** 0
--- 1,50
+ SPI::bootstrap();
+ use vars qw(%_SHARED);
+
+ sub ::plperl_warn {
+ (my $msg = shift) =~ s/\(eval \d+\) //g;
+ &elog(&NOTICE, $msg);
+ }
+ $SIG{__WARN__} = \&::plperl_warn;
+
+ sub ::plperl_die {
+ (my $msg = shift) =~ s/\(eval \d+\) //g;
+ die $msg;
+ }
+ $SIG{__DIE__} = \&::plperl_die;
+
+ sub ::mkunsafefunc {
+ my $ret = eval(
