Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
Excerpts from Alex Hunsaker's message of sáb feb 12 04:53:14 -0300 2011: - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Incidentally, I think this bit was lost, no? -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 17, 2011 at 16:18, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Alex Hunsaker's message of sáb feb 12 04:53:14 -0300 2011: - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Incidentally, I think this bit was lost, no? It was, yes. -- 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] arrays as pl/perl input arguments [PATCH]
Excerpts from Alvaro Herrera's message of mié feb 16 19:54:07 -0300 2011: I cleaned up the patch a bit -- result is v11, attached. I'll give it another look tomorrow and hopefully commit it. Applied. Thanks. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
On Sat, Feb 12, 2011 at 02:17:12AM +0200, Alexey Klyukin wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). Given that encode_array_literal() encodes an _array_ as a literal, I'd assume encode_type_literal() encodes a _type_ as a literal. I'd suggest encode_typed_literal() as a better name. Tim [just passing though] -- 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] arrays as pl/perl input arguments [PATCH]
Excerpts from Tim Bunce's message of mié feb 16 14:08:11 -0300 2011: On Sat, Feb 12, 2011 at 02:17:12AM +0200, Alexey Klyukin wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). Given that encode_array_literal() encodes an _array_ as a literal, I'd assume encode_type_literal() encodes a _type_ as a literal. I'd suggest encode_typed_literal() as a better name. FYI I'm looking at this patch (v10), and I'll incorporate Tim's suggestion. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
I cleaned up the patch a bit -- result is v11, attached. I'll give it another look tomorrow and hopefully commit it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support pg_to_perl_arrays_v11.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Feb 16, 2011 at 12:21, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Tim Bunce's message of mié feb 16 14:08:11 -0300 2011: I'd suggest encode_typed_literal() as a better name. FYI I'm looking at this patch (v10), and I'll incorporate Tim's suggestion. Looks good to me. -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 12, 2011, at 9:53 AM, Alex Hunsaker wrote: On Fri, Feb 11, 2011 at 17:17, Alexey Klyukin al...@commandprompt.com wrote: Anyway in playing with this patch a bit more I found another bug return [[]]; would segfault. So find attached a v9 that: - fixes above segfault - made plperl_sv_to_literal vastly simpler (no longer uses SPI and calls regtypin directly) - removed extraneous /para added in v8 in plperl.sgml (my docbook stuff is broken, so I can't build them, hopefully there are no other problems) - we now on the fly (when its requested) make the backwards compatible string for arrays (it was a few lines now that we have plperl_sv_to_literal) - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Anyway barring any more bugs, I think this patch is ready. pg_to_perl_arrays_v9.patch.gz Thank you for finding and fixing the bugs there. I think removing the para was a mistake. I specifically added it to fix the problem I had when building the docs: openjade:plperl.sgml:353:8:E: end tag for PARA omitted, but OMITTAG NO was specified openjade:plperl.sgml:201:2: start tag was here After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. pg_to_perl_arrays_v10.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Best, David -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 7:45 PM, David E. Wheeler wrote: On Feb 15, 2011, at 6:39 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 15, 2011, at 9:49 AM, Alexey Klyukin wrote: After I re-added the closing /para in plperl.sgml:235 these errors disappeared, and the resulting html looks fine too. v10 with just this single change is attached. So is this ready for committer? Yes. Awesom, thanks Alexey Alex! David -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 10, 2011, at 11:26 PM, Alexey Klyukin wrote:
On Feb 10, 2011, at 9:44 PM, Andrew Dunstan wrote:
On 02/10/2011 08:15 AM, Alexey Klyukin wrote:
Let me try implementing that as an XS interface to plperl_array_to_datum.
Are you intending this as a completion of the current patch or as 9.2 work?
If the former you need to send it in real fast.
I'd like to extend the current patch, going to post the update by tomorrow.
So, here is the v8. Instead of rewriting the encode_array_literal I've added
another function, encode_type_literal (could use a better name). basically a
user-level interface to plperl_sv_to_datum, which accepts the perl variable
and the type name as a text string and returns the string representation of
the input variable according to the output function of the argument type, e..g:
do $$ elog(NOTICE, encode_type_literal([[1],[2],[3]], 'int[]'));$$ language
plperl;
NOTICE: {{1},{2},{3}}
I can easily convert the encode_array_literal to just call this function, but
not encode_array_constructor, since the latter needs to produce its own string
representation, different from the result of the type output function. One
option would be traversing through the SV *, duplicating the efforts of
plperl_sv_to_datum, but I actually wonder what do we need the
encode_array_constructor (and encode_array_literal) functions for ? I
guess they were useful to pass array references to SPI, but per my
understanding it's possible to use perl array references in SPI functions
directly now, so are there any other use cases for these functions, which
justify the time to spend on updating them to support arrays of composites
(and shouldn't we also provide encode_composite_literal and
encode_composite_constructor as well) ?
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
pg_to_perl_arrays_v8.patch.gz
Description: GNU Zip compressed data
--
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] arrays as pl/perl input arguments [PATCH]
On Fri, Feb 11, 2011 at 17:17, Alexey Klyukin al...@commandprompt.com wrote: So, here is the v8. Instead of rewriting the encode_array_literal I've added another function, encode_type_literal (could use a better name). ... I can easily convert the encode_array_literal to just call this function, but not encode_array_constructor, but I actually wonder what do we need the encode_array_constructor (and encode_array_literal) functions for ? I guess they were useful to pass array references to SPI, I dunno, Ill have to go dig through the archives. but per my understanding it's possible to use perl array references in SPI functions directly now, so are there any other use cases for these functions, which justify the time to spend on updating them to support arrays of composites Probably not, but I dunno if we can just drop them out from people using them... (and shouldn't we also provide encode_composite_literal and encode_composite_constructor as well) ? Well we should not need encode_composite_literal, encode_type_literal() should work for that. I don't see a reason for the _constructor variant so id vote against it unless there is a use case. Anyway in playing with this patch a bit more I found another bug return [[]]; would segfault. So find attached a v9 that: - fixes above segfault - made plperl_sv_to_literal vastly simpler (no longer uses SPI and calls regtypin directly) - removed extraneous /para added in v8 in plperl.sgml (my docbook stuff is broken, so I can't build them, hopefully there are no other problems) - we now on the fly (when its requested) make the backwards compatible string for arrays (it was a few lines now that we have plperl_sv_to_literal) - make plperl.o depend on plperl_helpers.h (should have been done in the utf8 patch) Anyway barring any more bugs, I think this patch is ready. pg_to_perl_arrays_v9.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukin al...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On 02/10/2011 08:15 AM, Alexey Klyukin wrote: On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukinal...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. Are you intending this as a completion of the current patch or as 9.2 work? If the former you need to send it in real fast. 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] arrays as pl/perl input arguments [PATCH]
On Feb 10, 2011, at 9:44 PM, Andrew Dunstan wrote: On 02/10/2011 08:15 AM, Alexey Klyukin wrote: On Feb 9, 2011, at 9:28 PM, Alex Hunsaker wrote: On Wed, Feb 9, 2011 at 08:24, Alexey Klyukinal...@commandprompt.com wrote: What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. Yeah, It does not work because it did not take into account the order of composite columns. It always put them alphabetically by column name. To do it properly we would need to pass in a typid or a column order or something. Ideally we could expose the new plperl_array_to_datum() to plperl functions in some manner. Damn, right. Each perl hash corresponds to multiple composite types, different by the order of the type elements. Passing the typid sounds like a fair requirement (and if it's missing we could assume that the order of columns in composites doesn't matter to the caller). Let me try implementing that as an XS interface to plperl_array_to_datum. Are you intending this as a completion of the current patch or as 9.2 work? If the former you need to send it in real fast. I'd like to extend the current patch, going to post the update by tomorrow. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Feb 9, 2011, at 3:44 AM, Alex Hunsaker wrote: So the merge while not exactly trivial was fairly simple. However it would be great if you could give it another look over. Find attached v7 changes include: - rebased against HEAD - fix potential use of uninitialized dims[cur_depth] - took out accidental (broken) hack to try and support composite types in ::encode_array_literal (added in v4 or something) - make_array_ref() now uses plperl_hash_from_datum() for composite types instead of its own hand rolled version - get_perl_array_ref() now grabs the 'array' directly instead of through the magic interface for simplicity - moved added static declarations to the bottom instead of being half on top and half on bottom pg_to_perl_arrays_v7.patch.gz Thank you very much, the new patch applies cleanly and passes all tests on my system. The new get_perl_array_ref seems to be much more clear to me, than the prev. magic call. What was actually broken in encode_array_literal support of composite types (it converted perl hashes to the literal composite-type constants, expanding nested arrays along the way) ? I think it would be a useful extension of the existing encode_array_literal. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 08, 2011 at 09:40:38AM -0500, Andrew Dunstan wrote: On 02/03/2011 01:20 PM, Andrew Dunstan wrote: Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. do you till have concerns about this, or are you happy for us to move ahead on it? [I'm not really paying close enough attention for you to put much weight on my opinions, but...] I can't see any major issues so I'm happy for you to move ahead. Thanks! 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] arrays as pl/perl input arguments [PATCH]
On Wed, Feb 9, 2011 at 08:24, Alexey Klyukin al...@commandprompt.com wrote:
What was actually broken in encode_array_literal support of composite types
(it converted perl hashes to the literal composite-type constants, expanding
nested arrays along the way) ? I think it would be a useful extension of the
existing encode_array_literal.
Yeah, It does not work because it did not take into account the order
of composite columns. It always put them alphabetically by column
name. To do it properly we would need to pass in a typid or a column
order or something. Ideally we could expose the new
plperl_array_to_datum() to plperl functions in some manner.
Here is a longer perhaps more concrete example:
Imagine you have a composite type with two 'columns':
= create type foo as (z int, a int);
= create or replace function foo_pl(foo[]) returns foo[] as $$
my $arg = shift;
$$ language plperl;
= select foo_pl('{(1,2), (3,4)}');
In the above $arg looks something like (ignoring the
PostgreSQL::InServer::ARRAY object) [{'a'=2, 'z'=1}, {'a'=4,
'z'=3}]. When we call encode_arary_literal() we need to put it back
in to composite literal form which is basically (ignoring the array)
(column_z, column_a). However without type information we don't
know the order of the columns, as the composite is represented as a
hash we get kind of stuck. The hack I did sorted the hash keys
alphabetically, which worked for the regression tests as they happened
to have their composite columns sorted alphabetically. But would break
for this example putting $arg-[0]{a} into z and $arg-[0]{z} into a.
--
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] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 01:20 PM, Andrew Dunstan wrote: - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a string) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. I'd missed that it was using the internal array representation (obvious in hindsight) but there's still a significant cost in allocating the SVs that won't be used by existing code. Though I agree it's of the same order as for composite types. Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. Tim, do you till have concerns about this, or are you happy for us to move ahead on it? 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] arrays as pl/perl input arguments [PATCH]
On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: On Thu, Feb 3, 2011 at 18:29, Alex Hunsaker bada...@gmail.com wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. For example: ... On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash: So here is a v6 that does the above. It does so by providing a generic plperl_sv_to_datum() function and converting the various places to use it. One cool thing is you can now use the composite types or arrays passed in (or returned from another spi!) as arguments for spi_exec_prepared(). Before you would have to convert the hashes to strings. It also provides a real plperl_convert_to_pg_array (now plperl_array_to_datum) that can handle composite and other random datatypes. This also means we don't have to convert arrays to a string representation first. (which removes part of the argument for #5 @ http://archives.postgresql.org/pgsql-hackers/2011-02/msg00197.php) I have attached a stand alone version of the above so its easier to look over. The diffstat is fairly small (ignoring added regression tests) 1 files changed, 259 insertions(+), 165 deletions(-) Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: So here is a v6 Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Well, you can't AV *av = (AV *)SvRV(sv); And the SV * amagic_call returns is already a reference, so the newRV_noinc() would be redundant no? It occurs to me instead of doing the amagic_call we could just call the to_array method directly using perl_call_pv(). That would look more normal and less magic-- thats probably a good thing? Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). Good catch, will fix. All of those checks should be outside of the while loop. While Im at it Ill also rebase against HEAD (im sure there will be some conflicts with that other plperl patch that just went in ;)). -- 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] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker bada...@gmail.com wrote: On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Well, you can't AV *av = (AV *)SvRV(sv); And the SV * amagic_call returns is already a reference, so the newRV_noinc() would be redundant no? It occurs to me instead of doing the amagic_call we could just call the to_array method directly using perl_call_pv(). That would look more normal and less magic-- thats probably a good thing? Err, even simpler would be to just access the 'array' member directly out of the hash object. -- 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] arrays as pl/perl input arguments [PATCH]
On Tue, Feb 8, 2011 at 10:33, Alex Hunsaker bada...@gmail.com wrote: On Tue, Feb 8, 2011 at 08:18, Alexey Klyukin al...@commandprompt.com wrote: On Feb 6, 2011, at 9:43 AM, Alex Hunsaker wrote: So here is a v6 Comments? Thanks, looks great to me. It passes all the tests on my OS X system. I wonder what's the purpose of the amagic_call in get_perl_array_ref, instead of calling newRV_noinc on the target SV * ? Err, even simpler would be to just access the 'array' member directly out of the hash object. Done as the above, an added bonus is we no longer have to SvREF_dec() so its even simpler. Also, in array_to_datum (line 1050), if get_perl_array_ref returns NULL for the first element of the corresponding dimension, but non-NULL for the second one - it would use uninitialized dims[cur_depth] value in comparison (which, although, would probably lead to the desired comparison result). Good catch, will fix. All of those checks should be outside of the while loop. I clearly needed more caffeine in my system when i wrote that. Partly due to the shadowed av variable. I've fixed it by initiating all of the dims to 0. I also renamed the shadowed av var to nav for nested av. While Im at it Ill also rebase against HEAD (im sure there will be some conflicts with that other plperl patch that just went in ;)). So the merge while not exactly trivial was fairly simple. However it would be great if you could give it another look over. Find attached v7 changes include: - rebased against HEAD - fix potential use of uninitialized dims[cur_depth] - took out accidental (broken) hack to try and support composite types in ::encode_array_literal (added in v4 or something) - make_array_ref() now uses plperl_hash_from_datum() for composite types instead of its own hand rolled version - get_perl_array_ref() now grabs the 'array' directly instead of through the magic interface for simplicity - moved added static declarations to the bottom instead of being half on top and half on bottom pg_to_perl_arrays_v7.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 3, 2011 at 18:29, Alex Hunsaker bada...@gmail.com wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. For example: ... On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash: So here is a v6 that does the above. It does so by providing a generic plperl_sv_to_datum() function and converting the various places to use it. One cool thing is you can now use the composite types or arrays passed in (or returned from another spi!) as arguments for spi_exec_prepared(). Before you would have to convert the hashes to strings. It also provides a real plperl_convert_to_pg_array (now plperl_array_to_datum) that can handle composite and other random datatypes. This also means we don't have to convert arrays to a string representation first. (which removes part of the argument for #5 @ http://archives.postgresql.org/pgsql-hackers/2011-02/msg00197.php) I have attached a stand alone version of the above so its easier to look over. The diffstat is fairly small (ignoring added regression tests) 1 files changed, 259 insertions(+), 165 deletions(-) Comments? plperl_uniform_inout.patch.gz Description: GNU Zip compressed data pg_to_perl_arrays_v6.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 08:29 PM, Alex Hunsaker wrote: On Mon, Jan 31, 2011 at 01:34, Alexey Klyukinal...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. One thing I find odd is we allow for nested arrays, but not nested composites. This is not unique to the patch, it how nested composites are handled in general. That is if you passed in ROW(ROW()), the first ROW would be a hash while the 2nd row would be a scalar. On the other end, the same is true when returning. If you try to return a nested composite or array, the child better be a string as it wont let you pass a hash I think the reason this is so has to do with the history. Composite support came to plperl about the same time (in the 8.0 cycle, IIRC) that we were getting more support for nested composites in Postgres, and they sorta passed like ships in the night. Anyone object to fixing the above as part of this patch? That is making plperl_(build_tuple_result, modify_tuple, return_next, hash_from_tuple) handle array and hash (composite/row) types consistently? And _that_ would be to recurse and turn them from/into perl objects. Thoughts? I have no objection to making the whole thing work recursively, in principle, but will it break legacy code? 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] arrays as pl/perl input arguments [PATCH]
On Fri, Feb 4, 2011 at 10:29, Andrew Dunstan and...@dunslane.net wrote:
Anyone object to fixing the above as part of this patch? That is
making plperl_(build_tuple_result, modify_tuple, return_next,
hash_from_tuple) handle array and hash (composite/row) types
consistently? And _that_ would be to recurse and turn them from/into
perl objects. Thoughts?
I have no objection to making the whole thing work recursively, in
principle, but will it break legacy code?
It will certainly change how nested composites are represented on the
'input side'. I would argue its a bug the way it is now and also
violates the POLA. I think we should also remain backwards compatible
on the output side so you could still return a string:
-- how things are currently, manually assigning a composite-literal
(would continue to work)
= create or replace function trigger_func() returns trigger as $$
$_TD-{'new'}{'nested_composite'} = {'a'='(1,2)'}';
return 'MODIFY';
-- it would also work with perl nested structures (currently broken)
= create or replace function trigger_func() returns trigger as $$
$_TD-{'new'}{'nested_composite'} = {'a'={'b'=1, 'c'=2}};
return 'MODIFY';
$$
Or perhaps you are saying we should do something similar with what we
now do with arrays? The pseudo array dance that is.
--
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] arrays as pl/perl input arguments [PATCH]
Hi,
On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote:
I'm sorry I'm late to this party. I haven't been keeping up with
pgsql-hackers.
Better late than never :)
I'd kind'a hoped that this functionality could be tied into extending
PL/Perl to handle named arguments. That way the perl variables
corresponding to the named arguments could be given references without
breaking any code.
Franky I don't see a direct connection between conversion of arrays into array
references and supporting named arguments. Could you, please, show some
examples of how you hoped the functionality could be extended?
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
In principal, I think that's not hard to built with the functionality provided
by this patch. I see this as an XS function which takes the array string,
calls the array_in to convert it to the array datum, and, finally, calls
plperl_ref_from_pg_array (provided by the patch) to produce the resulting
array reference.
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
Well, per my understanding of Alex changes, the string parsing is not invoked
unless requested by referencing the array in a string context. Normally, onle
plperl_ref_from_pg_array will be invoked every time the function is called
with an array argument, which would take little time to convert the PostgreSQL
internal array representation (not a sting) to the perl references, but that's
no different from what is already done with composite type arguments, which
are converted to perl hash references on every corresponding function call.
- Some of those functions may not use the array at all and some may
simply pass it on as an argument to another function.
I don't see how it would be good to optimize for the functions that are
declared to get the array but in fact do nothing with it. And considering the
case of passing an array through to another function, it's currently not
possible to call another pl/perl function from an existing one directly, and
when passing muti-dimensional arrays to a perl function one would need to
convert it to the array references anyway.
- Making the conversion lazy would be a big help.
Converting it to string is already lazy, and, per the argumens above, I don't
see a real benefit of lazy conversion to the perl reference (as comparing to
the hurdles of implementing that).
/A
--
Alexey Klyukin
The PostgreSQL Company - Command Prompt, Inc.
--
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] arrays as pl/perl input arguments [PATCH]
On Feb 3, 2011, at 4:23 AM, Alexey Klyukin wrote: - Making the conversion lazy would be a big help. Converting it to string is already lazy, and, per the argumens above, I don't see a real benefit of lazy conversion to the perl reference (as comparing to the hurdles of implementing that). I agree that we should prefer an actual array as the implementation and lazily provide a string when needed. Best, David -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 03, 2011 at 02:23:32PM +0200, Alexey Klyukin wrote:
Hi,
On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote:
I'm sorry I'm late to this party. I haven't been keeping up with
pgsql-hackers.
Better late than never :)
I'd kind'a hoped that this functionality could be tied into extending
PL/Perl to handle named arguments. That way the perl variables
corresponding to the named arguments could be given references without
breaking any code.
Franky I don't see a direct connection between conversion of arrays
into array references and supporting named arguments.
There is no direct connection. I wasn't clear, tied was too strong a
word for it.
Could you, please, show some examples of how you hoped the
functionality could be extended?
I wasn't suggesting extending the functionality, just a way of adding
the functionality that wouldn't risk impacting existing code.
Imagine that PL/Perl could handle named arguments:
CREATE FUNCTION join_list( separator text, list array ) AS $$
return join( $separator, @$list );
$$ LANGUAGE plperl;
The $list variable, magically created by PL/Perl, could be the array
reference created by your code, without altering the contents of @_.
Existing code that does the traditional explicit unpacking of @_
wouldn't be affected. So there would be no need for the string overload
hack which, although I suggested it, I'm a little uncomfortable with.
(The problems with recursion and the need for is_array_ref with
hardwired class names are a little troubling. Not to mention the
extra overheads in accessing the array.)
On the ther hand, a string version of the array would still need to be
created for @_.
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
In principal, I think that's not hard to built with the functionality provided
by this patch. I see this as an XS function which takes the array string,
calls the array_in to convert it to the array datum, and, finally, calls
plperl_ref_from_pg_array (provided by the patch) to produce the resulting
array reference.
I think that would be useful.
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
Well, per my understanding of Alex changes, the string parsing is not invoked
unless requested by referencing the array in a string context. Normally, onle
plperl_ref_from_pg_array will be invoked every time the function is called
with an array argument, which would take little time to convert the PostgreSQL
internal array representation (not a string) to the perl references, but
that's
no different from what is already done with composite type arguments, which
are converted to perl hash references on every corresponding function call.
I'd missed that it was using the internal array representation (obvious
in hindsight) but there's still a significant cost in allocating the SVs
that won't be used by existing code. Though I agree it's of the same
order as for composite types.
- Some of those functions may not use the array at all and some may
simply pass it on as an argument to another function.
I don't see how it would be good to optimize for the functions that are
declared to get the array but in fact do nothing with it. And considering the
case of passing an array through to another function, it's currently not
possible to call another pl/perl function from an existing one directly, and
when passing muti-dimensional arrays to a perl function one would need to
convert it to the array references anyway.
I was thinking of calls to other pl/perl functions via sql.
- Making the conversion lazy would be a big help.
Converting it to string is already lazy, and, per the argumens above, I don't
see a real benefit of lazy conversion to the perl reference (as comparing to
the hurdles of implementing that).
I (now) agree. Thanks.
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] arrays as pl/perl input arguments [PATCH]
On 02/03/2011 01:01 PM, Tim Bunce wrote: Imagine that PL/Perl could handle named arguments: CREATE FUNCTION join_list( separator text, list array ) AS $$ return join( $separator, @$list ); $$ LANGUAGE plperl; The $list variable, magically created by PL/Perl, could be the array reference created by your code, without altering the contents of @_. I think that's getting way too subtle, and it would probably violate the POLA. If we implement named arguments I would expect $list to be the same as $_[0] - Every existing plperl function that takes arrays is going to get slower due to the overhead of parsing the string and allocating the array and all its elements. Well, per my understanding of Alex changes, the string parsing is not invoked unless requested by referencing the array in a string context. Normally, onle plperl_ref_from_pg_array will be invoked every time the function is called with an array argument, which would take little time to convert the PostgreSQL internal array representation (not a string) to the perl references, but that's no different from what is already done with composite type arguments, which are converted to perl hash references on every corresponding function call. I'd missed that it was using the internal array representation (obvious in hindsight) but there's still a significant cost in allocating the SVs that won't be used by existing code. Though I agree it's of the same order as for composite types. Well, the question seems to be whether or not it's a reasonable price to pay. On the whole I'm inclined to think it is, especially when it can be avoided by updating your code, which will be a saving in fragility and complexity as well. 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] arrays as pl/perl input arguments [PATCH]
On tor, 2011-02-03 at 18:01 +, Tim Bunce wrote: Imagine that PL/Perl could handle named arguments: CREATE FUNCTION join_list( separator text, list array ) AS $$ return join( $separator, @$list ); $$ LANGUAGE plperl; The $list variable, magically created by PL/Perl, could be the array reference created by your code, without altering the contents of @_. I would find that pretty surprising, since Perl itself doesn't even provide named arguments. -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Feb 3, 2011 at 05:23, Alexey Klyukin al...@commandprompt.com wrote:
Hi,
On Feb 2, 2011, at 7:16 PM, Tim Bunce wrote:
[...]
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
In principal, I think that's not hard to built with the functionality provided
by this patch. I see this as an XS function which takes the array string,
*cough* array string and Oid for the datatype right? :P
calls the array_in to convert it to the array datum, and, finally, calls
plperl_ref_from_pg_array (provided by the patch) to produce the resulting
array reference.
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
Well, per my understanding of Alex changes, the string parsing is not invoked
unless requested by referencing the array in a string context.
Sorry, there does seem to be some confusion here. The first version I
posted did lazy conversion to a string using encode_array_literal().
Unfortunately that function falls over with row/composite types. I
don't see a way to quote those without knowing the datatype, so in
interest of having it be 'correct' instead of fast, I made it always
decode to an array ref _and_ string in the next version :-(.
I see a couple of ways to fix this:
1) Using PTR2IV store the datum pointer in the array pseudo object.
This way when used as a string we have the original Datum and can call
the correct OutputFunction on demand. It would also let us call
plperl_ref_from_pg_array lazily. I have not actually tried it, but it
should work.
2) Add encode_composite_literal($hashref, $typeoid) and store the
type/Oid in the pseudo object. We could then call that lazily from the
perl.
3) Add the column position to the pseudo object and quote
appropriately. Simpler than #1 or #2 but not as robust.
4) Maybe there is a way of setting composite columns by column instead
of by position? If so we can encode that way. However everything on
http://www.postgresql.org/docs/current/interactive/rowtypes.html that
has to do with the 'literal' format seems to be positional.
5) Decided its worth the performance hit to always decode to both. (
Note it may not be as big as people think, in the case that you return
the array reference we have to convert it to a string anyway )
--
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] arrays as pl/perl input arguments [PATCH]
On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote:
I've looked at the patch and added a test for arrays exceeding or equal
maximum dimensions to check, whether the recursive function won't bring
surprises there. I've also added check_stack_depth calls to both split_array
and plperl_hash_from_tuple. Note that the regression fails currently due to
the incorrect error reporting in
PostgreSQL, per
http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php.
The v5 is attached.
One thing I find odd is we allow for nested arrays, but not nested
composites. For example:
= create type foo as (a int[]);
= create type foofoo as (b foo);
= create or replace function fa(foofoo[]) returns void as $$
my $foofoo_array = shift;
warn ref $foofoo_array-[0] || 'SCALAR';
warn ref $foofoo_array-[0]-{'b'} || 'SCALAR';
$$ language plperl;
= select fa(ARRAY[ROW(ROW(ARRAY[1]))]::foofoo[]);
WARNING: HASH at line 3.
CONTEXT: PL/Perl function fa
WARNING: SCALAR at line 4.
CONTEXT: PL/Perl function fa
Why is foofoo.b a scalar but foofoo is a hash? This is not unique to
the patch, it how nested composites are handled in general. That is
if you passed in ROW(ROW()), the first ROW would be a hash while the
2nd row would be a scalar.
On the other end, the same is true when returning. If you try to
return a nested composite or array, the child better be a string as it
wont let you pass a hash:
= create type foo as (a int[]);
= create or replace function foo_in(foo) returns foo as $$ shift; $$
language plperl;
= create or replace function foo_out() returns foo as $$ return
{'a'=[1,2,3]}; $$ language plperl;
= -- this works with the patch because we treat the reference as a string
= select foo_in('({1,2,3})'::foo);
foo_in
-
({1,2,3})
= select foo_out();
ERROR: array value must start with { or dimension information
CONTEXT: PL/Perl function foo_out
STATEMENT: SELECT foo_out();
-- also works as a straight string
= create or replace function foo_out_str() returns foo as $$ return
{'a'='{1,2,3}'}; $$ language plperl;
= select foo_out_str();
foo_out_str
-
({1,2,3})
(1 row)
Anyone object to fixing the above as part of this patch? That is
making plperl_(build_tuple_result, modify_tuple, return_next,
hash_from_tuple) handle array and hash (composite/row) types
consistently? And _that_ would be to recurse and turn them from/into
perl objects. Thoughts?
--
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] arrays as pl/perl input arguments [PATCH]
I'm sorry I'm late to this party. I haven't been keeping up with pgsql-hackers.
I'd kind'a hoped that this functionality could be tied into extending
PL/Perl to handle named arguments. That way the perl variables
corresponding to the named arguments could be given references without
breaking any code.
Some observations on the current code (based on a quick skim):
- I'd like to see the conversion function exposed as a builtin
$ref = decode_array_literal({...});
- Every existing plperl function that takes arrays is going to get
slower due to the overhead of parsing the string and allocating the
array and all its elements.
- Some of those functions may not use the array at all and some may
simply pass it on as an argument to another function.
- Making the conversion lazy would be a big help.
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] arrays as pl/perl input arguments [PATCH]
On Mon, Jan 31, 2011 at 01:34, Alexey Klyukin al...@commandprompt.com wrote: I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. Looks good. Marked as Ready for committer -- 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] arrays as pl/perl input arguments [PATCH]
On Jan 29, 2011, at 12:27 AM, Alex Hunsaker wrote: On Thu, Jan 27, 2011 at 03:38, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. Wait a second... Just who is reviewing who's patch? :P Oh, sorry, got confused along the way :) Both done in the attached. I also renamed is_array() to is_array_ref() for clarity (hopefully). pg_to_perl_arrays_v4.patch.gz Thank you. I've looked at the patch and added a test for arrays exceeding or equal maximum dimensions to check, whether the recursive function won't bring surprises there. I've also added check_stack_depth calls to both split_array and plperl_hash_from_tuple. Note that the regression fails currently due to the incorrect error reporting in PostgreSQL, per http://archives.postgresql.org/pgsql-hackers/2011-01/msg02888.php. The v5 is attached. pg_to_perl_arrays_v5.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 27, 2011 at 03:38, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. Wait a second... Just who is reviewing who's patch? :P Both done in the attached. I also renamed is_array() to is_array_ref() for clarity (hopefully). pg_to_perl_arrays_v4.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
Hi, On Jan 27, 2011, at 9:31 AM, Alex Hunsaker wrote: Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Nice improvement. It passes all the regression tests on my OS X system. I have only a minor suggestion, I think is_array is worth mentioning in the utility functions chapter of the pl/perl documentation, it would be also more clear to use it in regression tests as opposed to manually checking whether the ref is equal to 'PostgreSQL::InServer::ARRAY'. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: 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). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. -- 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] arrays as pl/perl input arguments [PATCH]
Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: 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). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), but it still segfaults after I fixed that. Other than that, the approach looks good to me, I'm for eliminating the GUC setting in favor of it. /A -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: 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). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), Yeah, I noticed that after I sent it out :(. but it still segfaults after I fixed that. I seem to recall fixing this post email as well... Can you provide the function that broke so I can double check? (Or was it part of the regression test?) Thanks for taking the time to play with it. -- 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] arrays as pl/perl input arguments [PATCH]
On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: On Wed, Jan 26, 2011 at 12:44, Alexey Klyukin al...@commandprompt.com wrote: Hi, On Jan 26, 2011, at 8:45 PM, Alex Hunsaker wrote: On Sat, Jan 15, 2011 at 15:48, Alex Hunsaker bada...@gmail.com wrote: On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote: 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). Find attached a proof of concept that modifies Alexey's patch to do the above (using the overload example I and others posted). [ ... ] Thoughts? Should I polish this a bit more? Or do we like the GUC better? So its been over a week with no comments. ISTM there were more people against adding yet another GUC. Barring objection ill finish the missing parts of the POC patch I posted and submit that. I've played with that patch just today. I found a problem with it, when I tried to use the array in a string context the backend segfaulted with: WARNING: Deep recursion on subroutine main::encode_array_literal at -e line 74 just before the segfault. I think the problem is in the regexp check in 'encode_array_literal' (it's obviously reversed comparing with the original one), Yeah, I noticed that after I sent it out :(. but it still segfaults after I fixed that. I seem to recall fixing this post email as well... Can you provide the function that broke so I can double check? (Or was it part of the regression test?) Sure, it's really simple (and the plperl_array regressions tests exposes this problem as well): CREATE OR REPLACE FUNCTION test_array(INTEGER[]) RETURNS TEXT AS $$ my $array = shift; print $array.\n; $$ LANGUAGE plperl; /A I will look into this problem tomorrow unless you'll be lucky to fix it before that. Thank you for the review and your patch. -- Alexey Klyukin The PostgreSQL Company - Command Prompt, Inc. -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 26, 2011 at 13:35, Alexey Klyukin al...@commandprompt.com wrote: On Jan 26, 2011, at 10:08 PM, Alex Hunsaker wrote: (it's obviously reversed comparing with the original one), but it still segfaults after I fixed that. Ahh yep, the reason reversing the check did not fix it is order of operations. I had this fixed, but I had some unrelated changes in my tree. So I manually git add(ed) the plperl files so I could use git diff --cached to make the diff. Then I fixed this issue, but forgot to git-add the changes :(. That explains why make installcheck worked for me, but the diff I made was broken. If you add some parens around ref it should work: if ref($arg) !~ /ARRAY/; btw the next version I plan on posting will check more explicitly: if ref($arg) !~ /^(?:ARRAY|PostgreSQL::InServer::ARRAY)$/; -- 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] arrays as pl/perl input arguments [PATCH]
Find attached v3 of the patch. changes include: - fix deep recursion due to accidental reversal of check in encode_array_literal - add proper support for stringifying composite/row types. I did not find a good way to quote these from the perl on the fly, so instead we compute it the same way we used to and store the string inside the new object along with the array :(. - misc whitespace and code touchups pg_to_perl_arrays_v3.patch.gz Description: GNU Zip compressed data -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 13:04, Alexey Klyukin al...@commandprompt.com wrote:
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).
Find attached a proof of concept that modifies Alexey's patch to do
the above (using the overload example I and others posted).
Arrays have a reference of 'PostgreSQL::InServer::ARRAY'-- mainly to
be consistent with the other PL/Perl packages we have. It also lets
you match ref() =~ m/ARRAY$/ to make it a tad easier to figure out if
something is an array.
The other thing to note is I only applied this to the 'top' array.
That is when you have a multidimensional array, its children are
'real' arrays:
create or replace function takes_array(text[]) returns void as $$
my $array = shift;
elog(NOTICE, ref $array);
elog(NOTICE, ref $_) for (@$array);
$$ language plperl;
select takes_array('{{1}, {2, 3}}'::text[]);
NOTICE: PostgreSQL::InServer::ARRAY
CONTEXT: PL/Perl function takes_array
NOTICE: ARRAY
CONTEXT: PL/Perl function takes_array
NOTICE: ARRAY
CONTEXT: PL/Perl function takes_array
We could change that so _all_ arrays have a ref of
PostgreSQL::InServer::ARRAY. However I thought it would be nice to
easily use built-ins, this way you can just 'cast' away just the top
level without having to recurse to children (my @arr = @$array).
This also will create a string representation if and when you try to
use it as a string. It currently lacks row support in the stringify
case (and I left the regression test Alexey added for that failing) so
we would need to fix that up if we want to go down this road.
Thoughts? Should I polish this a bit more? Or do we like the GUC better?
pg_to_perl_arrays_kind_of.patch.gz
Description: GNU Zip compressed data
--
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] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 12:06:33AM -0700, Alex Hunsaker wrote:
I had supposed that it would be possible to do the string conversion
lazily, ie, only if the string value was actually demanded.
Yep, In-fact if we wanted we could even die (or throw an exception in
other language speak :) ) when the string value is demanded.
I played with this a little and it is fairly easy to make a variable
such that $a is the string representation and $a[0] the first value of
the array. The problem is that you can't pass such a variable into a
subroutine.
I was thinking however, if the parameters if the function have names
you can use, then you can make it work. $_[0] would still go the old
way, but the named parameters could be the array.
== cut ==
#!/usr/bin/perl -w
use strict;
no strict 'vars';
package MyClass;
sub TIESCALAR {
my $class = shift;
my $self = shift;
return bless $self, $class;
}
sub FETCH {
my $self = shift;
return join(,, @$self);
}
my @a=(1,2);
tie $a, MyClass, \@a;
print \$a='$a'\n;
print \$a[0]='$a[0]'\n;
--
Martijn van Oosterhout klep...@svana.org http://svana.org/kleptog/
Patriotism is when love of your own people comes first; nationalism,
when hate for people other than your own comes first.
- Charles de Gaulle
signature.asc
Description: Digital signature
Re: [HACKERS] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 01:06, Martijn van Oosterhout klep...@svana.org wrote: On Thu, Jan 13, 2011 at 12:06:33AM -0700, Alex Hunsaker wrote: I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. Yep, In-fact if we wanted we could even die (or throw an exception in other language speak :) ) when the string value is demanded. I played with this a little and it is fairly easy to make a variable such that $a is the string representation and $a[0] the first value of the array. The problem is that you can't pass such a variable into a subroutine. [ snip ] my @a=(1,2); tie $a, MyClass, \@a; print \$a='$a'\n; print \$a[0]='$a[0]'\n; Erm... the reason you can't seem to pass it to any subroutines is its actually 2 variables: $a, @a. When you print $a\n; you are using the tied variable that uses @a; And when you print $a[0]\n; you are accessing the array directly. I think you just had an unfortunate variable name, otherwise strict would have complained appropriately. :) -- 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] arrays as pl/perl input arguments [PATCH]
On Thu, Jan 13, 2011 at 2:06 AM, Martijn van Oosterhout
klep...@svana.org wrote:
I played with this a little and it is fairly easy to make a variable
such that $a is the string representation and $a[0] the first value of
the array. The problem is that you can't pass such a variable into a
subroutine.
I played with this too:
#!/usr/bin/perl -w
use strict;
package Pg::ArrayArg;
use overload
''= \as_s,
'@{}' = \as_a;
sub new {
my $proto = shift;
my $class = ref $proto || $proto;
bless {
string = shift,
array = shift
}, $class;
}
sub as_s {
shift-{ 'string' };
}
sub as_a {
shift-{ 'array' };
}
package main;
my $aa = Pg::ArrayArg-new( '{1,2}', [ 1, 2 ] );
printf ref = %s\n, ref $aa;
print string = $aa\n;
printf string = %s\n, $aa;
printf array index = (%s, %s)\n, $aa-[ 0 ], $aa-[ 1 ];
printf array_ref = %s\n, scalar @$aa;
print regexp test = ;
if ($aa =~ /^{(.*)}$/) {
print looks like array\n;
printf join of split = %s\n, join ';', split /,/, $1;
} else {
print doesn't look like array\n;
}
Suppose one of these compatibility objects is passed into legacy code
as $_[0]. The problem is that 'ref $_[0]' will return 'Pg::ArrayArg'
instead of what it used to, '' (empty string). Other than that, I
think it performs as people would expect.
You could even change 'as_s' to generate the string on the fly as
requested instead of generating both representations at instantiation.
Just my $0.02.
--
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] arrays as pl/perl input arguments [PATCH]
On Jan 13, 2011, at 4:15 PM, Stephen J. Butler wrote: Suppose one of these compatibility objects is passed into legacy code as $_[0]. The problem is that 'ref $_[0]' will return 'Pg::ArrayArg' instead of what it used to, '' (empty string). Other than that, I think it performs as people would expect. Well, frankly, since up to this patch you *never* got an ARRAY reference argument, who would be calling `ref` on it anyway? You could even change 'as_s' to generate the string on the fly as requested instead of generating both representations at instantiation. Yep. Best, David -- 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] arrays as pl/perl input arguments [PATCH]
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. -- 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] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 4:06 AM, Robert Haas wrote: On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan and...@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. -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin al...@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) ] -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 12:22:55PM -0700, Alex Hunsaker wrote: On Wed, Jan 12, 2011 at 06:34, Alexey Klyukin al...@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 da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@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 -- 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] arrays as pl/perl input arguments [PATCH]
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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
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. -- 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] arrays as pl/perl input arguments [PATCH]
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. -- 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] arrays as pl/perl input arguments [PATCH]
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 alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] arrays as pl/perl input arguments [PATCH]
Alexey Klyukin al...@commandprompt.com writes: 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. IMO a GUC for this completely sucks, because if you do need to convert your Perl functions, the only way to do it is to have a flag day wherein they all change at once. And what about people writing Perl functions that they'd like to give to other people? If you have to have a flag, the only useful sort of flag is one that can be attached to individual functions. Compare what we did for plpgsql.variable_conflict in 9.0. I don't know how practical that will be in plperl, though. I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. regards, tom lane -- 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] arrays as pl/perl input arguments [PATCH]
On 01/12/2011 04:22 PM, Tom Lane wrote: Alexey Klyukinal...@commandprompt.com writes: 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. IMO a GUC for this completely sucks, because if you do need to convert your Perl functions, the only way to do it is to have a flag day wherein they all change at once. And what about people writing Perl functions that they'd like to give to other people? If you have to have a flag, the only useful sort of flag is one that can be attached to individual functions. Compare what we did for plpgsql.variable_conflict in 9.0. I don't know how practical that will be in plperl, though. I don't see why it should be terribly difficult. We have the source code and we have a couple of powerful regex engines. Determining it it has a string in some position like # pragma: plperl.arrays_as_strings doesn't seem onerous. It's just a SMOC. It's not too hard to imagine other things that might be useful for. I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 4:45 PM, Andrew Dunstan and...@dunslane.net wrote: I thought the idea of overloading the string representation to look like the old style was a cute solution. If we don't have anyone at hand who knows how to do that, let's find someone who does. Not break our users' code because we're too lazy to find out how to do it properly. What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. I agree with your gut feeling. I suspect the Perl magic solution will make no one very happy. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] arrays as pl/perl input arguments [PATCH]
On Jan 12, 2011, at 3:29 PM, Robert Haas wrote: What I was casting a bit of doubt on upthread was whether or not this would work without possibly breaking some code, in possibly silent or obscure ways. If I'm wrong about that, then by all means let's use some perl Magic (that's a technical term) to achieve this. IIRC Alex recently posted some code that might be instructive about this. I agree with your gut feeling. I suspect the Perl magic solution will make no one very happy. Could do both, I suppose… David -- 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] arrays as pl/perl input arguments [PATCH]
David E. Wheeler da...@kineticode.com writes: 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. I'm not seeing how an unsupported fear that there *might* be some incompatibilities is a good argument for instead adopting an approach that absolutely, positively, guaranteed *WILL* break everybody's code. regards, tom lane -- 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] arrays as pl/perl input arguments [PATCH]
David E. Wheeler da...@kineticode.com writes: 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. I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. regards, tom lane -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 22:12, Tom Lane t...@sss.pgh.pa.us wrote: David E. Wheeler da...@kineticode.com writes: 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. I had supposed that it would be possible to do the string conversion lazily, ie, only if the string value was actually demanded. Yep, In-fact if we wanted we could even die (or throw an exception in other language speak :) ) when the string value is demanded. -- 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] arrays as pl/perl input arguments [PATCH]
On Wed, Jan 12, 2011 at 14:45, Andrew Dunstan and...@dunslane.net wrote:
What I was casting a bit of doubt on upthread was whether or not this would
work without possibly breaking some code, in possibly silent or obscure
ways.
I can't see how it would break, unless we did it wrong...
If I'm wrong about that, then by all means let's use some perl Magic
(that's a technical term) to achieve this. IIRC Alex recently posted some
code that might be instructive about this.
There might be a more gutsy way to do this so that 'ref' gives you
back what you expected (would that be 'ARRAY' or 'SCALAR' ?), but
there is a simple pure perl solution using overload:
package PLPerl::ArgArray;
use overload '' = \to_str;
sub new
{
# note we bless an arrayref here instead of the usual hashref so
# you can use this 'object' as a normal array
return bless([@_], ... );
}
sub to_str
{
my $self = shift;
# yeah this is not right not correct :P
# we could also die here with something like You are trying to use
an Array as a string or whatever
return join(',', map { '{'. $_ .'}' } @{$self});
}
-
--
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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
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 -- 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] arrays as pl/perl input arguments [PATCH]
On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstan and...@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 -- 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] arrays as pl/perl input arguments [PATCH]
On 01/11/2011 09:06 PM, Robert Haas wrote: On Tue, Jan 11, 2011 at 7:55 PM, Andrew Dunstanand...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
