4
\$\begingroup\$

I have a node.js/express.js based REST application. In one GET service I am querying data based on different set of request parameters. I am looking for a better way to implement it.

function handleGet(connection,req,res) {

var sql = 'SELECT id, name, age, rent, marks FROM student WHERE class = 10';

if (req.param('minAge') && req.param('maxAge')) {
    sql = sql + ' AND age BETWEEN ' + connection.escape(req.param('minAge')) + ' AND ' + connection.escape(req.param('maxAge'));
}

if (req.param('minRent') && req.param('maxRent')) {
    sql =sql + ' AND rent BETWEEN ' + connection.escape(req.param('minRent')) 
        + ' AND ' + connection.escape(req.param('maxRent'));
};

if (req.param('marks')) {
    sql = sql + ' AND marks >= ' + connection.escape(req.param('marks'));
};

};



var limit = ('undefined' === typeof req.params.limit) ? 20: req.params.limit;

connection.query(sql + ' ORDER BY id DESC LIMIT ' + limit, function handleSql(err, rows){
        if (err){ logAndRespond(err,res); return; }
        if (rows.length === 0){ res.send(204); return; }
        res.json(200, {'students':rows});
        connection.release();
});
}

Data can be queried by passing difference combination request parameters. Can anyone please suggest any better (reusable, abstract, robust) way of doing it? I was wondering if I can create a module for building query. Something like setting all param values in the module and let the module validate and construct the query.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Most I've seen are similar to yours, they just begin with select * from <table> WHERE 1 = 1 .... and class = 10.... etc. Also remember to use parameterized queries to protect against injection. \$\endgroup\$
    – Uncle Iroh
    Commented Jul 2, 2014 at 23:43
  • \$\begingroup\$ Take a loot at squel.js SQL query builder for Node.js. \$\endgroup\$
    – cvng
    Commented Jul 2, 2014 at 23:45
  • 1
    \$\begingroup\$ Maybe this helps: stackoverflow.com/questions/16182652/… \$\endgroup\$ Commented Jul 3, 2014 at 9:19

3 Answers 3

2
\$\begingroup\$

Instead of dynamic string concatenation, you can do a logic trick. I'm going to use named sql placeholders for clarity. You'll obviously need to implement the code, I'm just giving you the idea.

select a
     , b
     , c
  from someTbl
 where (:PMaxAge  is null or maxAge <= :PMaxAge)
   and (:PMinRent is null or rent   >= :PMinRent)
   and (:PMaxRent is null or rent   <= :PMaxRent)
   -- etc ...

You may need to check for an empty string instead of checking for null to detect when a param was omitted depending on you implementation, but you get the point - let each criteria be satisfied if the param is missing.

\$\endgroup\$
2
\$\begingroup\$

As @200_success suggested, don't use the keyword BETWEEN in SQL, the result set can be unpredictable due to it, especially with DATE or DATETIME data. Mathematical operators > < = are better.

You could significantly simplify your SQL-related code by not concatenating it with your Javascript code. Instead, I suggest to create a stored PROCEDURE in SQL and just pass parameters to it to get your result set. This will also improve performance. Here is an example (hint: this needs executed only once, after that it is stored):

CREATE PROCEDURE usp_getStudent
    @class INT = 10,
    @minAge INT,
    @maxAge INT,
    @minRent DECIMAL(6,2),
    @maxRent DECIMAL(6,2),
    @marks -- it is not clear what type these would be
AS
-- procedure usp_getStudent starts here
SET NOCOUNT ON;
SELECT id, name, age, rent, marks 
FROM student
WHERE
-- class filter
    class = @class
-- age filter
AND
    (
    age >= @minAge
    OR @minAge ISNULL
    )
AND
    (
    age <= @maxAge
    OR @maxAge ISNULL
    )
-- rent filter
AND
    (
    rent >= @minRent
    OR @minAge ISNULL
    )
AND
    (
    rent <= @maxRent
    OR @maxAge ISNULL
    )
-- marks filter
AND
    marks -- insert some condition here
;
GO

Then from Javascript all you need to do, instead of all this concatenating SQL code, is this:

sql = sql 'CALL usp_getStudent (value1, value2, value3, value4, value5, value6);'
\$\endgroup\$
1
\$\begingroup\$

I think it's counterintuitive that both minAge and maxAge have to be specified to have any effect. Why shouldn't the criteria work independently? Simplifying your code would also make your API less surprising. Don't bother with BETWEEN ? AND ?; go with separate >= and <= criteria instead. (Your goal is to produce the most predictable SQL query, not necessarily the prettiest.)

If possible, I'd rename the marks parameter to minMarks. Then your code can be more regular. (In fact, you would get maxMarks handling for free.)

var criteria = ['class = 10'];

var attrs = ['Age', 'Rent', 'Marks'];
attrs.forEach(function(attr) {
    if (req.param('min' + attr)) {
        criteria.push(attr.toLowerCase() + ' >= ' + connection.escape(req.param('min' + attr)));
    }
    if (req.param('max' + attr)) {
        criteria.push(attr.toLowerCase() + ' <= ' + connection.escape(req.param('max' + attr)));
    }
});

var limit = ('undefined' === typeof req.params.limit) ? 20 : req.param('limit');
var sql = 'SELECT id, name, age, rent, marks FROM student WHERE ' +
    attrs.join(' AND ') +
    ' ORDER BY id DESC LIMIT ' + limit;
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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