arrays as pl/perl input arguments [PATCH]
Hello,
Here's the patch that improves handling of arrays as pl/perl function input
arguments, converting postgres arrays of arbitrary dimensions into perl array
references. It includes regression tests and a documentation changes, and it
builds and runs successfully on my mac os x and linux boxes. To maintain
compatibility with existing pl/perl code a new variable,
plperl.convert_array_arguments (better name?), is introduced. Its default
value is false, when set to true it triggers the new behavior, i.e.
SET plperl.convert_array_arguments = true;
CREATE OR REPLACE FUNCTION test_array(text[]) RETURNS TEXT AS $$
my $arg = shift;
if (ref $arg eq 'ARRAY') {
return "array conversion is enabled";
} else {
return "array conversion is disabled";
}
$$ LANGUAGE plperl;
test=# select test_array('{1,2,3}');
test_array
-----------------------------
array conversion is enabled
(1 row)
You can find other, less trivial examples, by examining plperl_array
regression test.
The implementation detects whether the input argument of a perl function is an
array, and calls plperl_ref_from_pg_array if it is. The latter obtains a flat
array of Datums from deconstruct_array and, using information about array
dimensions, recursively creates perl array references in split_array. To pass
array information between recursive calls a new 'plperl_array_info' struct was
added. Arrays as members of composite types are also handled in
plperl_hash_from_tuple.
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
pg_to_perl_arrays.diffapplication/octet-stream; name=pg_to_perl_arrays.diffDownload+592-27
On Jan 11, 2011, at 2:25 PM, Alexey Klyukin wrote:
Hello,
Here's the patch that improves handling of arrays as pl/perl function input
arguments, converting postgres arrays of arbitrary dimensions into perl array
references.
Awesome! I've wanted this for *years*.
It includes regression tests and a documentation changes, and it
builds and runs successfully on my mac os x and linux boxes. To maintain
compatibility with existing pl/perl code a new variable,
plperl.convert_array_arguments (better name?), is introduced. Its default
value is false, when set to true it triggers the new behavior, i.e.
Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC.
Not my idea, BTW, but suggested by Tim Bunce.
Best,
David
On 01/11/2011 06:07 PM, David E. Wheeler wrote:
To maintain
compatibility with existing pl/perl code a new variable,
plperl.convert_array_arguments (better name?), is introduced. Its default
value is false, when set to true it triggers the new behavior, i.e.Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC.
Not my idea, BTW, but suggested by Tim Bunce.
I think there's at least a danger of breaking legacy code doing that.
Say you have some code that does a ref test on the argument, for
example. The behavior would now be changed.
cheers
andrew
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote:
I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed.
I think that'd be pretty rare.
David
On 01/11/2011 07:17 PM, David E. Wheeler wrote:
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote:
I think there's at least a danger of breaking legacy code doing that. Say you have some code that does a ref test on the argument, for example. The behavior would now be changed.
I think that'd be pretty rare.
Possibly it would. But we usually try pretty hard to avoid that sort of
breakage.
cheers
andrew
On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
On 01/11/2011 07:17 PM, David E. Wheeler wrote:
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote:
I think there's at least a danger of breaking legacy code doing that. Say
you have some code that does a ref test on the argument, for example. The
behavior would now be changed.I think that'd be pretty rare.
Possibly it would. But we usually try pretty hard to avoid that sort of
breakage.
By the same token, I'm not convinced it's a good idea for this
behavior to be off by default. Surely many people will altogether
fail to notice that it's an option? If we're going to have a
backward-compatibility GUC at all, ISTM that you ought to get the good
stuff unless you ask for the old way.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 01/11/2011 09:06 PM, Robert Haas wrote:
On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan<andrew@dunslane.net> wrote:
On 01/11/2011 07:17 PM, David E. Wheeler wrote:
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote:
I think there's at least a danger of breaking legacy code doing that. Say
you have some code that does a ref test on the argument, for example. The
behavior would now be changed.I think that'd be pretty rare.
Possibly it would. But we usually try pretty hard to avoid that sort of
breakage.By the same token, I'm not convinced it's a good idea for this
behavior to be off by default. Surely many people will altogether
fail to notice that it's an option? If we're going to have a
backward-compatibility GUC at all, ISTM that you ought to get the good
stuff unless you ask for the old way.
Sure, that seems reasonable.
cheers
andrew
On Jan 12, 2011, at 1:07 AM, David E. Wheeler wrote:
On Jan 11, 2011, at 2:25 PM, Alexey Klyukin wrote:
Hello,
Here's the patch that improves handling of arrays as pl/perl function input
arguments, converting postgres arrays of arbitrary dimensions into perl array
references.Awesome! I've wanted this for *years*.
It includes regression tests and a documentation changes, and it
builds and runs successfully on my mac os x and linux boxes. To maintain
compatibility with existing pl/perl code a new variable,
plperl.convert_array_arguments (better name?), is introduced. Its default
value is false, when set to true it triggers the new behavior, i.e.Have you considered instead passing an array-based object with is string overloading designed to return the pg array string format? That would make for nice, transparent compatibility without the need for a GUC.
You mean packing both a string representation and a reference to a single SV * value?
I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily.
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
On Jan 12, 2011, at 4:06 AM, Robert Haas wrote:
On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
On 01/11/2011 07:17 PM, David E. Wheeler wrote:
On Jan 11, 2011, at 3:44 PM, Andrew Dunstan wrote:
I think there's at least a danger of breaking legacy code doing that. Say
you have some code that does a ref test on the argument, for example. The
behavior would now be changed.I think that'd be pretty rare.
Possibly it would. But we usually try pretty hard to avoid that sort of
breakage.By the same token, I'm not convinced it's a good idea for this
behavior to be off by default. Surely many people will altogether
fail to notice that it's an option? If we're going to have a
backward-compatibility GUC at all, ISTM that you ought to get the good
stuff unless you ask for the old way.
I think the number of people failing to notice the changes would be the same whenever we set the new or the old behavior by default. I decided to default to the the old behavior since it won't break the existing code as opposed to just hiding the good stuff, although it would slower the adoption of the new behavior.
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote:
You mean packing both a string representation and a reference to a single SV * value?
Dunno, I'm not a guts guy.
I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily.
Andrew's objections have merit. So perhaps just add this patch to the commit fest as is?
Best,
David
On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin <alexk@commandprompt.com> wrote:
On Jan 12, 2011, at 4:06 AM, Robert Haas wrote:
By the same token, I'm not convinced it's a good idea for this
behavior to be off by default. Surely many people will altogether
fail to notice that it's an option? If we're going to have a
backward-compatibility GUC at all, ISTM that you ought to get the good
stuff unless you ask for the old way.I think the number of people failing to notice the changes would be the same whenever we set the new or the old behavior by default. I decided to default to the the old behavior since it won't break the existing code as opposed to just hiding the good stuff, although it would slower the adoption of the new behavior.
Personally, I think the point of a compatibility GUC is that at some
point in the distant future we can get rid of it. If we default to
the old behavior thats going to be harder to do. +1 for defaulting to
the new behavior.
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]
On Jan 12, 2011, at 11:22 AM, Alex Hunsaker wrote:
Personally, I think the point of a compatibility GUC is that at some
point in the distant future we can get rid of it. If we default to
the old behavior thats going to be harder to do. +1 for defaulting to
the new behavior.
+1
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]
Yeah, but the removal of Safe.pm actually *improved* compatibility. This patch, without the GUC, would break it.
Best,
David
On Wed, Jan 12, 2011 at 12:22:55PM -0700, Alex Hunsaker wrote:
On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin <alexk@commandprompt.com> wrote:
On Jan 12, 2011, at 4:06 AM, Robert Haas wrote:
By the same token, I'm not convinced it's a good idea for this
behavior to be off by default. �Surely many people will
altogether fail to notice that it's an option? �If we're going to
have a backward-compatibility GUC at all, ISTM that you ought to
get the good stuff unless you ask for the old way.I think the number of people failing to notice the changes would
be the same whenever we set the new or the old behavior by
default. I decided to default to the the old behavior since it
won't break the existing code as opposed to just hiding the good
stuff, although it would slower the adoption of the new behavior.Personally, I think the point of a compatibility GUC is that at some
point in the distant future we can get rid of it. If we default to
the old behavior thats going to be harder to do. +1 for defaulting
to the new behavior.[ Id actually vote for _not_ having a compatibility option at all,
we change more major things than this IMHO every major release. (And
even then some major things in minor releases, for example the
removal of Safe.pm) ]
+1 for changing the behavior to something sane with loud, specific
warnings in the release notes about what will break and how.
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Excerpts from Alex Hunsaker's message of mié ene 12 16:22:55 -0300 2011:
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]
I think the main question here is: how loudly is existing code going to
break? If the breakage is silent, it's going to be very problematic.
If functions fail to run at all, then we can live without the
compatibility option.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Jan 12, 2011, at 11:36 AM, Alvaro Herrera wrote:
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]I think the main question here is: how loudly is existing code going to
break? If the breakage is silent, it's going to be very problematic.
If functions fail to run at all, then we can live without the
compatibility option.
I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like "ARRAY(0x118ee2a0)" and return an empty array, or a NULL.
Best,
David
Excerpts from David E. Wheeler's message of mié ene 12 16:39:56 -0300 2011:
On Jan 12, 2011, at 11:36 AM, Alvaro Herrera wrote:
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]I think the main question here is: how loudly is existing code going to
break? If the breakage is silent, it's going to be very problematic.
If functions fail to run at all, then we can live without the
compatibility option.I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like "ARRAY(0x118ee2a0)" and return an empty array, or a NULL.
I kinda doubt that a function failing in that way would pass any
testing.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Jan 12, 2011, at 11:51 AM, Alvaro Herrera wrote:
I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like "ARRAY(0x118ee2a0)" and return an empty array, or a NULL.
I kinda doubt that a function failing in that way would pass any
testing.
What is this “testing” thing of which you speak?
David
On Jan 12, 2011, at 8:52 PM, David E. Wheeler wrote:
On Jan 12, 2011, at 5:14 AM, Alexey Klyukin wrote:
You mean packing both a string representation and a reference to a single SV * value?
Dunno, I'm not a guts guy.
Well, neither me (I haven't used much of the guts api there).
I haven't considered that (lack of extensive perlgus-foo) although I think that's an interesting idea. One drawback would be that it would require both conversion to a string format and to a perl reference, performing unnecessary actions during every time arrays are passed to a pl/perl function. If there is a strong dislike of the proposed 'compatibility' GUC option - I think I can change the existing patch to incorporate array string representation into the reference-holding SV quite easily.
Andrew's objections have merit. So perhaps just add this patch to the commit fest as is?
Already done :)
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
On Jan 12, 2011, at 9:36 PM, Alvaro Herrera wrote:
Excerpts from Alex Hunsaker's message of mié ene 12 16:22:55 -0300 2011:
[ Id actually vote for _not_ having a compatibility option at all, we
change more major things than this IMHO every major release. (And even
then some major things in minor releases, for example the removal of
Safe.pm) ]I think the main question here is: how loudly is existing code going to
break? If the breakage is silent, it's going to be very problematic.
If functions fail to run at all, then we can live without the
compatibility option.
Not really loud. Perl won't even complain when you try to interpret a
reference as a string.
Since almost everyone votes for making the new behavior a default option I'm
inclined to do that change, although I'm against throwing out the
compatibility option. There are many other reasons except for PL/Perl for
people to upgrade to 9.1, let's not force them to rewrite their Perl code
if they were not planning to do that.
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
Excerpts from David E. Wheeler's message of mié ene 12 16:55:17 -0300 2011:
On Jan 12, 2011, at 11:51 AM, Alvaro Herrera wrote:
I suspect it'd be quiet, unfortunately, since there are a bazillion ad hoc implementations of a Perl SQL array parser, and many of them, I suspect, don't complain if the string doesn't look like an SQL array. They would just parse a string like "ARRAY(0x118ee2a0)" and return an empty array, or a NULL.
I kinda doubt that a function failing in that way would pass any
testing.What is this “testing” thing of which you speak?
Ha ha ha :-(
I wonder if there's a way to overload the string representation of the
array so that it throws an error.
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
