Closed GoogleCodeExporter closed 9 years ago
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
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
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:
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
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
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
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
> 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
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
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
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
Original issue reported on code.google.com by
AMDuns...@gmail.com
on 19 Feb 2013 at 1:43