Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How can I write a better database query wrapper?

//code for connection 
exports.connect = function(callback) {
    client.connect(function(err, conInfo){
        if (err) callback(err);
        conInfo = "Connection at " + client.host + ":" + client.port + "/"
            + client.database + " used by " + client.user;
        callback(null, conInfo);
    });
};

//for queries that do not return realation
exports.runQuery = function(query_str, columns, params, callback) {
    var sqlQuery = query_str + " " + columns;
    client.query(sqlQuery, params, function(err) {
        console.log("NEW QUERY____________________");
        console.log(sqlQuery);
        console.log(params);
        if (err) throw err;
        callback(null);
    });
};

//for select queries
exports.executeQuery = function(query_str, callback) {
    var query = client.query(query_str, function(err){
        console.log("NEW QUERY____________________");
        console.log(query_str);
        if (err) throw err;
        query.on("row", function(row, result) {
            result.addRow(row);
        });
        query.on("end", function (result) {
            callback(null, result.rows);
        });
    });
};
share|improve this question

1 Answer 1

up vote 2 down vote accepted

I don't do JS, even less node.js, but from the looks of it you seem to be taking in a string for your query, which means client code can do something like "select x, y, sum(z) from mytable where x = " + someValue + " group by x, y" - which is prone to SQL-injection: your client code needs to remember that.

If your executeQuery also took parameters, I think it would be clearer that when you need parameters, they should be passed as arguments to that function, not concatenated into the query string.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.