TryGhost / node-sqlite3

SQLite3 bindings for Node.js
BSD 3-Clause "New" or "Revised" License
6.22k stars 815 forks source link

Unhandled null parameters #116

Closed jooadam closed 11 years ago

jooadam commented 11 years ago
var sqlite = require('sqlite3').verbose();
var db = new sqlite.Database('test.db');

db.all('SELECT * FROM test', null, function (error, rows) {
    if (error) {
        console.log(error);
    } else {
        rows.forEach(function (row) {
            console.log(row);
        });
    }
});

db.close();

The previous code will produce the following output:

{ [Error: SQLITE_RANGE: bind or column index out of range] errno: 25, code: 'SQLITE_RANGE' }

Changing the second argument of Database#all() from null to [] produces the desired output.

This handling of null values is both counterintuitive and undocumented. Other methods may be affected too.

kkaefer commented 11 years ago

Why are you passing a null value in the first place? This behavior is sort of intended; all parameters after the SQL query and before the callback functions are added as parameters to the query. Since your query doesn't have any parameters, you get this error. Using [] is an alternative syntax to passing the parameters directly and [null] will fail in a similar way. It doesn't matter what value you pass; db.all('SELECT * FROM test', 'test')` fails with the same error.

jooadam commented 11 years ago

I see now. I for one never really liked the idea of variable length argument lists, and it seemed natural to pass a null for the bind values. Actually, even the formal parameter list in the docs isn’t clear on this since the comma is outside of the brackets. It would be nice to state explicitly in the description that everything before the callback will be treated as bind values.

springmeyer commented 11 years ago

@jooadam - docs are a wiki, please edit as you see fit.

hems commented 9 years ago

lol, i can't believe adding [] was the trick. jesus!

hems commented 9 years ago

i did a quick note at https://github.com/mapbox/node-sqlite3/wiki/API let me know what you think @springmeyer !

peace!

:8ball:

kkaefer commented 9 years ago

looks good, @hems!

khelkun commented 3 years ago

Just to say this could have its place in the README too.

I wanted to use a callback on my db.run("CREATE TABLE lorem (info TEXT)") so I did:

db.run("CREATE TABLE lorem (info TEXT)", null, (err) => {
  if(err) return console.error("failed to create table", err);
  console.log("succeeded to create table");
});

This gives the following error output and actually fails to create the table:

Failed to create dummy table: [Error: SQLITE_RANGE: column index out of range] {
  errno: 25,
  code: 'SQLITE_RANGE'
}

It took me some time to understand the root cause of this error was the null second parameter of my db.run call. Obviously this does the job:

db.run("CREATE TABLE lorem (info TEXT)", [], (err) => {
  if(err) return console.error("failed to create table", err);
  console.log("succeeded to create table");
});