Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

column "nan" does not exist #310

Closed
ghazel opened this issue Aug 22, 2013 · 6 comments · Fixed by #318
Closed

column "nan" does not exist #310

ghazel opened this issue Aug 22, 2013 · 6 comments · Fixed by #318
Labels

Comments

@ghazel
Copy link

ghazel commented Aug 22, 2013

Using PostGres, when a value is set which cannot be interpreted as a Number, incorrect SQL is generated which tries to use NaN as a value:

Example:

var orm = require('orm');

orm.connect('postgres://user:pass@host/db?debug=true', function (err, db) {
  if (err) throw err;
  var fun = db.define('fun', {'count': Number});
  fun.sync(function (err) {
    if (err) throw err;
    fun.create([{'count': 'bugz'}], function (err, items) {
      if (err) throw err;
      console.log(item);
    });
  });
});

Causes:


(orm/postgres) CREATE TABLE "fun" ("id" SERIAL, "count" REAL, PRIMARY KEY ("id"))
(orm/postgres) INSERT INTO "fun" ("count") VALUES (NaN) RETURNING *

test.js:9
      if (err) throw err;
                     ^
error: column "nan" does not exist
    at Connection.parseE (pg/lib/connection.js:526:11)
    at Connection.parseMessage (pg/lib/connection.js:371:17)
    at Socket.<anonymous> (pg/lib/connection.js:86:20)
    at Socket.EventEmitter.emit (events.js:95:17)
    at Socket.<anonymous> (_stream_readable.js:736:14)
    at Socket.EventEmitter.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:408:10)
    at emitReadable (_stream_readable.js:404:5)
    at readableAddChunk (_stream_readable.js:165:9)
    at Socket.Readable.push (_stream_readable.js:127:10)

Yes, "bugz" can't be converted to a number, but the natural PostGres error is much more useful:

# INSERT INTO "fun" ("count") VALUES ('bugz');
ERROR:  invalid input syntax for type real: "bugz"
@dxg
Copy link
Collaborator

dxg commented Aug 22, 2013

I've added some code to deal with it here.

Please try:

rm node_modules/sql-query -rf
npm install dresende/node-sql-query

and see if that fixes it.

@ghazel
Copy link
Author

ghazel commented Aug 22, 2013

Why pass 'NaN' instead of the string which was passed in? Maybe postgres knows how to convert whatever it is.

@dxg
Copy link
Collaborator

dxg commented Aug 22, 2013

The code should only affect numbers.

If a number is invalid [NaN, Infinity], it gets converted to a string and you get a nice database error.

@ghazel
Copy link
Author

ghazel commented Aug 23, 2013

What I mean is, the query orm generates now is:

INSERT INTO "fun" ("count") VALUES ('NaN') RETURNING *

Instead of:

INSERT INTO "fun" ("count") VALUES ('bugz') RETURNING *

Which actually suppresses the error entirely, storing the postgres NaN value instead of throwing the error like it should.

Conceivably I could be passing a string which postgres interprets properly but which orm cannot, exactly like NaN; Infinity and -Infinity are also supported. Also postgres knows what to do with '0xff' while orm doesn't.

@dxg
Copy link
Collaborator

dxg commented Aug 23, 2013

Interestingly, this throws an error as expected:

    if (err) throw err;

    var item = new fun();
    item.count = 'bugz';

    item.save(function (err) {
      if (err) throw err;
      console.log(JSON.stringify(item));

      db.close();
    });

// (orm/postgres) INSERT INTO "fun" ("count") VALUES ('bugz') RETURNING *
// error: invalid input syntax for type real: "bugz"
//   at Connection.parseE (/root/projects/node-orm2/node_modules/pg/lib/connection.js:519:11)

Something else is casting the value but only in the create call, not when assigning directly.
I can't really look into into this now but will do so on the weekend if noone else does so by then.

Edit: You've inadvertantly stumbled onto another bug. I'm looking at it now.

@dxg
Copy link
Collaborator

dxg commented Aug 25, 2013

I've got a fix, just gotta fix some tests.
Edit: Tests fixed; writing more new tests to prevent regressions..

dxg added a commit that referenced this issue Aug 28, 2013
Number fixes for sqlite.
Should fix #310
@dxg dxg closed this as completed in #318 Aug 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants