Re: [HACKERS] Initial refactoring of plperl.c - rebased [PATCH]

2010-01-04 Thread Andrew Dunstan 



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]

2009-12-25 Thread Andrew Dunstan 



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]

2009-12-25 Thread Tim Bunce 
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]

2009-12-25 Thread Andrew Dunstan 



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]

2009-12-04 Thread Tim Bunce 
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]

2009-12-03 Thread Josh Berkus 
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