Array literal rejected for array parameter

[GoogleCodeExporter]

What steps will reproduce the problem?

do language plv8 $$ plv8.execute("select $1::int[]",['{1,2,3}']); $$;

What is the expected output? What do you see instead?

get:

ERROR:  Error: value is not an Array

should get no error

Please use labels and text to provide additional information.

Original issue reported on code.google.com by AMDuns...@gmail.com on 19 Feb 2013 at 1:43

[GoogleCodeExporter]

The correct way to do it is

do language plv8 $$ plv8.execute("select $1::int[]",[[1,2,3]]); $$;

and it works for me.  WontFix.

Original comment by umi.tan...@gmail.com on 20 Feb 2013 at 7:02

• Changed state: WontFix

[GoogleCodeExporter]

It seems completely non-orthogonal and a violation of POLA to reject literals 
for arrays when they are accepted elsewhere. Surely we could add an

   if (value->IsString())
   {
   }

section to ToArrayDatum().

We'd probably need to cache a little bit of extra info in the type structure to 
support it, but it seems quite possible in principle.

Original comment by AMDuns...@gmail.com on 20 Feb 2013 at 3:23

[GoogleCodeExporter]

here's a patch that shows what I'm talking about. It seems to work just fine.

Original comment by AMDuns...@gmail.com on 20 Feb 2013 at 7:25

Attachments:

• arraylit.patch

[GoogleCodeExporter]

I'm not convinced that your usage is consistent.  You never want to write 
postgres' array literal when you want to return array from plv8 function.  The 
proposal is just error-prone.

Original comment by umi.tan...@gmail.com on 21 Feb 2013 at 7:06

[GoogleCodeExporter]

I don't see what is error prone about it. 

The use case for this is that the code is adapted from some code that runs in 
client side code that does accept (and possibly requires) array literals. 
Insisting on not allowing this makes adapting this code quite a deal more 
complicated.

It's also consistent with what happens elsewhere. PL/Perl, for example, happily 
accepts both sorts of uses:

andrew=# do language plperlu $$ use Data::Dumper; $plan = spi_prepare('select 
$1[2] as x','text[]'); $rv = spi_exec_prepared($plan,'{a,b,c,d}'); 
elog(NOTICE,Dumper($rv->{rows}[0])); $$;
NOTICE:  $VAR1 = {
          'x' => 'b'
        };

CONTEXT:  PL/Perl anonymous code block
DO
andrew=# do language plperlu $$ use Data::Dumper; $plan = spi_prepare('select 
$1[2] as x','text[]'); $rv = spi_exec_prepared($plan,[qw(a b c d)]); 
elog(NOTICE,Dumper($rv->{rows}[0])); $$;
NOTICE:  $VAR1 = {
          'x' => 'b'
        };

Please reconsider your attitude to this. I'd be sorry to have to maintain a 
fork, which is what I will probably have to do otherwise.

Original comment by AMDuns...@gmail.com on 21 Feb 2013 at 10:03

[GoogleCodeExporter]

What is the client program are you referring to?  My old js client with Rhino 
had the same interface as plv8.  I wouldn't say that is the reason not to 
change the interface, but rather I'm a little dubious about matching the 
interface since your client program X might have different i/f from my Y.  Your 
proposal is not 100% upside, someone may misunderstandingly choose the literal 
style and pass unexpectedly (which is error prone).  I/F change is a big deal.  
To me plperl doesn't look nice but that might be the people's understanding.  
What about plpython?

Original comment by umi.tan...@gmail.com on 21 Feb 2013 at 3:38

[GoogleCodeExporter]

The code in question is node.js client code. The driver they are using is a 
fairly thin veneer over libpq, and doesn't appear to do any conversion of 
arrays when passing them to postgres, although it happily converts them into JS 
arrays when they are handed back by postgres.

Plpython appears not to allow for this, but so what? You are just asserting 
that it is error prone. Why is it? Why is it more error prone to allow a string 
to be passed as an array than as a timestamp,which we happily let through? This 
just seems to me completely non-orthogonal and inconsistent.

Original comment by AMDuns...@gmail.com on 21 Feb 2013 at 5:24

[GoogleCodeExporter]

> Plpython appears not to allow for this, but so what?

You said it's consistent what happens elsewhere.  There doesn't seem to 
consistency around different modules/interfaces.

I believe your change will affect the value conversion for returned value from 
functions.  If I'm using wrong terminology due to my lack of knowledge of 
English, it might not be "error-prone".  But I don't like returning JS string 
for int-array returning function.  If it happens, I'd like postgres say it's 
wrong and correct my program.

Original comment by umi.tan...@gmail.com on 21 Feb 2013 at 10:28

[GoogleCodeExporter]

It's not even consistent with treatment of other objects in plv8:

andrew=# do language plv8 $$ var jres = plv8.execute("select 
$1::timestamptz",['yesterday']);  plv8.elog(NOTICE,JSON.stringify(jres));$$;
NOTICE:  [{"timestamptz":"2013-02-21T05:00:00.000Z"}]

I don't get why arrays (and records) should be different.

If you don't want to pass a string back, then don't, this certainly doesn't 
force you to, but I don't see why you should want to prevent someone who does 
want to pass back a valid array literal for whatever reason from doing so.

Original comment by AMDuns...@gmail.com on 22 Feb 2013 at 2:59

[GoogleCodeExporter]

I don't want to so I don't, but the new behavior masks my mistake, which I 
don't like.  It's not only you that is affected.

That said, it is true that object/array handling is specialized and different 
from scalar values (which I still think is reasonable as they are different in 
fact).  Maybe there should be conversion routine for array and object and it 
may mitigate the current limitation of multi-dim array support.  However, your 
previous patch doesn't seem to handle object/record, though.  Also it doesn't 
consider output(datum to value).  So if you can complete both of object/record 
and output, I'll be happy to get it in.

Original comment by umi.tan...@gmail.com on 22 Feb 2013 at 5:17

[GoogleCodeExporter]

It won't mask any error, unless you accidentally pass in a valid array literal. 
How likely is that?

andrew=# do language plv8 $$ var jres = plv8.execute("select 
$1::rgb[]",['{red,green}']); plv8.elog(NOTICE,JSON.stringify(jres)); $$;
NOTICE:  [{"rgb":["red","green"]}]
DO
andrew=# do language plv8 $$ var jres = plv8.execute("select 
$1::rgb[]",['foo']); plv8.elog(NOTICE,JSON.stringify(jres)); $$;
ERROR:  Error: array value must start with "{" or dimension information

You're right about it not handling record literals. I'll add that and show you 
a new patch.

Original comment by AMDuns...@gmail.com on 22 Feb 2013 at 5:37