type coercion (string to int) fails

[GoogleCodeExporter]

This might not be a bug, but I'm having a rough go with type coercion errors so 
hopefully this can be fixed.

summary: (id is type integer)

plv8.execute("select * from typetest where id = '1'") -- works
plv8.execute("select * from typetest where id = $1",['1']) -- Error: operator 
does not exist: integer = text

detail:

CREATE TABLE typetest (id serial, name text);
INSERT INTO typetest(name) values('test');
do $$ plv8.elog(NOTICE,JSON.stringify(plv8.execute("select * from typetest 
where id = '1'"))) $$ language plv8;
do $$ plv8.elog(NOTICE,JSON.stringify(plv8.execute("select * from typetest 
where id = $1",['1']))) $$ language plv8;
DROP TABLE typetest;

output:

psql:db/plv8bug.sql:1: NOTICE:  CREATE TABLE will create implicit sequence 
"typetest_id_seq" for serial column "typetest.id"
CREATE TABLE
INSERT 0 1
psql:db/plv8bug.sql:3: NOTICE:  [{"id":1,"name":"test"}]
DO
psql:db/plv8bug.sql:4: ERROR:  Error: operator does not exist: integer = text
DETAIL:  undefined() LINE 1:  
plv8.elog(NOTICE,JSON.stringify(plv8.execute("select * from typetest where id
DROP TABLE

using plv8 latest trunk, pg 9.1.3

Original issue reported on code.google.com by t...@blit.com on 16 May 2012 at 8:27

[GoogleCodeExporter]

What do you expect from this example?  The current intention is that 
plv8.execute() doesn't take "type" arguments so it infers the PG type from JS 
type of the input values.  The intended way to work with your example is,

plv8.execute("select * from typetest where id = $1", [1])

to tell plv8 that the SQL parameter $1 is an integer by giving 1 not '1'.

Original comment by umi.tan...@gmail.com on 21 May 2012 at 5:35

[GoogleCodeExporter]

The example was to show that pg seems to be fine with coercing types, in that 

select * from typetest where id = '1'

works just as select * from typetest where id = 1

I understand your point about the entended use of the API, but I believe this 
to be a common use case when passing JSON as a parameter into a plv8 function.  
I haven't gotten to dates yet, but I'm assuming if an string representation of 
an integer will error, then a date string of '2012-05-16 15:24:51.195555-07' 
will not work without first calling new Date('2012-05-16 15:24:51.195555-07').

Postgres also has many types not available in js, 64 bit integers would need to 
be passed in as strings.

I think I remember having an issue with plv8 when using enums as well (getting 
an error when passing the enum value as a string).

Sorry for not being more concrete and test this out some more, but I'm right in 
the middle of hacking/testing some stuff with 9.2 and I haven't put plv8 back 
in yet and I'm loosing a battle with the clock to get the stakeholder to buy in 
on pg and js over some other alternatives.

Original comment by t...@blit.com on 21 May 2012 at 1:35

[GoogleCodeExporter]

Hmm, I see.  Let me take a closer look at it.

Original comment by umi.tan...@gmail.com on 22 May 2012 at 5:04

[GoogleCodeExporter]

In your example, the constant '1' is of type "unknown".  I think the this kind 
of problems have been there around like JDBC, and are not easy to solve (one 
way has pros and cons).  I believe plv8.execute should follow those external 
driver behavior to avoid surprises (you have a surprise here, though :)

If I recall correctly, in cases like JDBC, your repro should look like

select * from typetest where id = $1::int

with int cast for $1.

Original comment by umi.tan...@gmail.com on 29 May 2012 at 6:16

[GoogleCodeExporter]

I don't have any experience with JDBC, but node-postgres (the pg driver for 
node) handles the type coercion.  My goal is to port an app that currently uses 
node-postgres for all queries to plv8 and am writing a type of RESTful db 
access layer in plv8.

I ran the below queries with node-postgres:

pg.connect("tcp://postgress@localhost/testdb",function(err,client){
  client.query("select * from typetest where id = $1",['1'],function(err,result){
    console.log('STR::INT:',err,result);
  });

  client.query("select * from typetest where id = $1",['NAN'],function(err,result){
    console.log('NAN:',err,result);
  });
});

OUTPUT:

STR::INT: null { rows: [ { id: 1, name: 'test' } ],
  command: 'SELECT',
  rowCount: 1,
  oid: NaN }
NAN: { [error: invalid input syntax for integer: "NAN"]
  length: 85,
  name: 'error',
  severity: 'ERROR',
  code: '22P02',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  file: 'numutils.c',
  line: '62',
  routine: 'pg_atoi' }

What is interesting about this output is node-postgres knows it is binding to 
an int, without help from my code, and if I pass a string that can not be 
casted to an integer I get an error out of pg_atoi.  But I think node-postgres 
is just handling off to the postgres library and getting type coercion built in.

The plv8 error is "Error: operator does not exist: integer = text" so it 
appears that somebody knows the underlying type is an integer and the passed 
value is text and somwhere postgres knows to call pg_atoi on the value instead 
of throwing back an error.

We are probably going to work around this issue for now by querying pg_catalog 
and getting the type information and decorate each $x with the proper ::type, 
but that's probably going to slow it down a bit.

Original comment by t...@blit.com on 29 May 2012 at 5:38

[GoogleCodeExporter]

Just ran into the same issue with citext, only instead of an error I just get 
an empty result.  

I have a users table where the id column is of type citext:

plv8.execute("SELECT * from users where id=$1",['krazykrut']) -- RETURNS []

plv8.execute("SELECT * from users where id=$1",['KrazyKrut']) -- RETURNS 
[{id:"KrazyKrut", ...}]

plv8.execute("SELECT * from users where id=$1::citext",['krazykrut']) -- 
RETURNS [{id:"KrazyKrut", ...}]

So it seems that plv8 will not use the comparative properties of the underlying 
type unless explicitly told to do so. 

Original comment by t...@blit.com on 30 May 2012 at 12:40

[GoogleCodeExporter]

OK, I think I understand the difference between plv8 and node-postgres.

This is not only node-postgres, but client applications are allowed to omit 
parameter types when the statement is prepared.  In that case, the backend 
parser deduces each type of parameters and your example is exactly this case.  
If the client program including node-postgres gives explicit type information, 
the situation is same as plv8.

The problem is, however, this is not easy for plv8 which uses SPI instead of 
libpq.  All the SPI interfaces seem to prohibit NULL input to argtypes.  If 
given, SPI immediately returns with an error code.  An exception might be 
SPI_prepare_params, which lets the caller of SPI to hook the parameter 
resolution.  I need to try and see if it works as expected, but at least this 
interface was introduced in 9.0.  Personally, I'd like to make the behavior 
same since 8.4 to the latest version.  Now we're going to get 9.2, we could 
drop 8.4, though...

Original comment by umi.tan...@gmail.com on 30 May 2012 at 5:49

[GoogleCodeExporter]

I've just created a topic branch for this.  Can you update your git repository 
and checkout 'deduce_paramtype' branch?  It requires 9.1.  If the behavior is 
what you want, I'll clean up and wrap it up.

Original comment by umi.tan...@gmail.com on 31 May 2012 at 7:22

• Changed state: Accepted

[GoogleCodeExporter]

Thanks, works great.  Tested with int, timestamp, enum, text and citext!

Original comment by t...@blit.com on 31 May 2012 at 4:40

[GoogleCodeExporter]

Thanks for testing.  I'm thinking of the design aspect, as it's better to align 
the behavior with plv8.prepare() and cursors.  Soon I'll release v1.1.0 to 
PGXN, and after that I'll incorporate this along with the session reset issue, 
as these are kind of new features.

Original comment by umi.tan...@gmail.com on 1 Jun 2012 at 6:12

[GoogleCodeExporter]

Now it's on master branch.  Prepared plans also make use of it.
http://code.google.com/p/plv8js/source/detail?r=9904bc94cb9450d7e746d1f76a1ded96
fa060145

Original comment by umi.tan...@gmail.com on 26 Jun 2012 at 7:01

• Changed state: Fixed