Improve query parser to handle functions in where clauses

lib/adapter.js

@@ -13,5 +13,14 @@ module.exports = {
 	 */
 	execute: function(sql, params, callback) {
 		throw new Error("Correct adapter not defined for query execution");
+	},
+
+	/**
+	 * Close the connection that is open on the current adapter
+	 *
+	 * @return void
+	 */
+	close: function() {
+		throw new Error("Close method not defined for the current adapter");
 	}
 };

lib/driver.js

@@ -20,6 +20,8 @@ var d = {
 	 * @private
 	 */
 	_quote: function(str) {
+		//if (/[0-9]+|\'(.*?)\'/ig.test(str)) return str;
+
 		return (helpers.isString(str) && ! (str.startsWith(d.identifierChar) || str.endsWith(d.identifierChar)))
 			? d.identifierChar + str + d.identifierChar
 			: str;
@@ -71,6 +73,12 @@ var d = {
 			return str.map(d.quoteIdentifiers);
 		}
 
+if ( ! helpers.isString(str))
+{
+	console.error(str);
+	return str;
+}
+
 		// Handle commas
 		if (str.contains(','))
 		{

lib/query-builder.js

@@ -2,14 +2,15 @@
 
 /** @module query-builder */
 var getArgs = require('getargs'),
-	helpers = require('./helpers');
+	helpers = require('./helpers'),
+	State = require('./state');
 
 /**
  * Variables controlling the sql building
  *
  * @private
  */
-var state = {};
+var state = new State();
 
 /*
  * SQL generation object
@@ -155,10 +156,17 @@ var QueryBuilder = function(driver, adapter) {
 
 			var obj = {};
 
-			if (helpers.isScalar(args.$key) && !helpers.isUndefined(args.$val) && !helpers.isNull(args.$val))
+
+			if (helpers.isScalar(args.$key) && !helpers.isUndefined(args.$val))
 			{
+				// Convert key/val pair to a simple object
 				obj[args.$key] = args.$val;
 			}
+			else if (helpers.isScalar(args.$key) && helpers.isUndefined(args.$val))
+			{
+				// If just a string for the key, and no value, create a simple object with duplicate key/val
+				obj[args.$key] = args.$key;
+			}
 			else
 			{
 				obj = args.$key;
@@ -177,15 +185,17 @@ var QueryBuilder = function(driver, adapter) {
 				}
 			});
 
+
 			return state[args.$varName];
 		},
 		whereMixedSet: function(/*key, val*/) {
 			var args = getArgs('key:string|object, [val]', arguments);
 
 			state.whereMap = [];
+			state.rawWhereValues = [];
 
 			_p.mixedSet('whereMap', 'both', args.key, args.val);
-			_p.mixedSet('whereValues', 'value', args.key, args.val);
+			_p.mixedSet('rawWhereValues', 'value', args.key, args.val);
 		},
 		fixConjunction: function(conj) {
 			var lastItem = state.queryMap[state.queryMap.length - 1];
@@ -210,24 +220,16 @@ var QueryBuilder = function(driver, adapter) {
 			// Normalize key and value and insert into state.whereMap
 			_p.whereMixedSet(key, val);
 
-			Object.keys(state.whereMap).forEach(function(field) {
+			// Parse the where condition to account for operators,
-				// Split each key by spaces, in case there
+			// functions, identifiers, and literal values
-				// is an operator such as >, <, !=, etc.
+			state = parser.parseWhere(driver, state);
-				var fieldArray = field.trim().split(' ').map(helpers.stringTrim);
 
-				var item = driver.quoteIdentifiers(fieldArray[0]);
+			state.whereMap.forEach(function(clause) {
-
-				// Simple key value, or an operator?
-				item += (fieldArray.length === 1 || fieldArray[1] === '') ? '=?' : " " + fieldArray[1] + " ?";
-
-				// Determine the correct conjunction
 				var conj = _p.fixConjunction(defaultConj);
-
+				_p.appendMap(conj, clause, 'where');
-				_p.appendMap(conj, item, 'where');
-
-				// Clear the where Map
-				state.whereMap = {};
 			});
+
+			state.whereMap = {};
 		},
 		whereNull: function(field, stmt, conj) {
 			field = driver.quoteIdentifiers(field);
@@ -243,20 +245,15 @@ var QueryBuilder = function(driver, adapter) {
 			// Normalize key/val and put in state.whereMap
 			_p.whereMixedSet(args.key, args.val);
 
-			Object.keys(state.whereMap).forEach(function(field) {
+			// Parse the having condition to account for operators,
-				// Split each key by spaces, in case there
+			// functions, identifiers, and literal values
-				// is an operator such as >, <, !=, etc.
+			state = parser.parseWhere(driver, state);
-				var fieldArray = field.split(' ').map(helpers.stringTrim);
-
-				var item = driver.quoteIdentifiers(fieldArray[0]);
-
-				// Simple key value, or an operator?
-				item += (fieldArray.length === 1) ? '=?' : " " + fieldArray[1] + " ?";
 
+			state.whereMap.forEach(function(clause) {
 				// Put in the having map
 				state.havingMap.push({
 					conjunction: (state.havingMap.length > 0) ? " " + args.conj + " " : ' HAVING ',
-					string: item
+					string: clause
 				});
 			});
 
@@ -291,7 +288,7 @@ var QueryBuilder = function(driver, adapter) {
 				vals = state.values.concat(state.whereValues);
 			}
 
-//console.log(state.queryMap);
+//console.log(state);
 //console.log(sql);
 //console.log(vals);
 //console.log(callback);
@@ -314,28 +311,7 @@ var QueryBuilder = function(driver, adapter) {
 			return sql;
 		},
 		resetState: function() {
-			state = {
+			state = new State();
-				// Arrays/Maps
-				queryMap: [],
-				values: [],
-				whereValues: [],
-				setArrayKeys: [],
-				orderArray: [],
-				groupArray: [],
-				havingMap: [],
-				whereMap: {},
-
-				// Partials
-				selectString: '',
-				fromString: '',
-				setString: '',
-				orderString: '',
-				groupString: '',
-
-				// Other various values
-				limit: null,
-				offset: null
-			};
 		}
 	};
 
@@ -363,10 +339,14 @@ var QueryBuilder = function(driver, adapter) {
 		return state;
 	};
 
-	// ------------------------------------------------------------------------
+	/**
-
+	 * Closes the database connection for the current adapter
-	// Set up state object
+	 *
-	this.resetQuery();
+	 * @return void
+	 */
+	this.end = function() {
+		adapter.close();
+	};
 
 	// ------------------------------------------------------------------------
 	// ! Query Builder Methods

lib/query-parser.js

@@ -3,21 +3,39 @@
 var helpers = require('./helpers');
 
 var matchPatterns = {
-	'function': /([a-zA-Z0-9_]+\((.*?)\))/i,
+	'function': /([a-z0-9_]+\((.*)\))/i,
-	identifier: /([a-zA-Z0-9_\-]+\.?)+/ig,
+	operator: /\!=?|\=|\+|&&?|~|\|\|?|\^|\/|<>|>=?|<=?|\-|%|OR|AND|NOT|XOR/ig,
-	operator: /\=|AND|&&?|~|\|\|?|\^|\/|>=?|<=?|-|%|OR|\+|NOT|\!=?|<>|XOR/i
+	literal: /([0-9]+)|'(.*?)'|true|false/ig
 };
 
+// Full pattern for identifiers
+// Making sure that literals and functions aren't matched
+matchPatterns.identifier = new RegExp(
+	'('
+		+ '(?!'
+			+ matchPatterns['function'].source + '|'
+			+ matchPatterns.literal.source
+		+ ')'
+		+ '([a-z_\-]+[0-9]*\\.?)'
+	+ ')+'
+, 'ig');
+
 // Full pattern for determining ordering of the pieces
-matchPatterns.combined = new RegExp(matchPatterns['function'].source + "+|"
+matchPatterns.joinCombined = new RegExp(
+	matchPatterns['function'].source + "+|"
+	+ matchPatterns.literal.source + '+|'
 	+ matchPatterns.identifier.source
-	+ '|(' + matchPatterns.operator.source + ')+', 'ig');
+	+ '|(' + matchPatterns.operator.source + ')+'
+, 'ig');
+
+var identifierBlacklist = ['true','false','null'];
 
 var filterMatches = function(array) {
 	var output = [];
 
 	// Return non-array matches
-	if (helpers.isScalar(array) || helpers.isNull(array) || helpers.isUndefined(array)) return output;
+	if (helpers.isNull(array)) return null;
+	if (helpers.isScalar(array) || helpers.isUndefined(array)) return output;
 
 	array.forEach(function(item) {
 		output.push(item);
@@ -38,6 +56,17 @@ var QueryParser = function(driver) {
 	// That 'new' keyword is annoying
 	if ( ! (this instanceof QueryParser)) return new QueryParser(driver);
 
+	/**
+	 * Check if the string contains an operator, and if so, return the operator(s).
+	 * If there are no matches, return null
+	 *
+	 * @param {String} string - the string to check
+	 * @return {Array|null}
+	 */
+	this.hasOperator = function(string) {
+		return filterMatches(string.match(matchPatterns.operator));
+	}
+
 	/**
 	 * Tokenize the sql into parts for additional processing
 	 *
@@ -49,12 +78,13 @@ var QueryParser = function(driver) {
 		var output = {};
 
 		// Get clause components
-		matches['function'] = sql.match(matchPatterns['function']);
+		matches.functions = sql.match(new RegExp(matchPatterns['function'].source, 'ig'));
 		matches.identifiers = sql.match(matchPatterns.identifier);
 		matches.operators = sql.match(matchPatterns.operator);
+		matches.literals = sql.match(matchPatterns.literal);
 
 		// Get everything at once for ordering
-		matches.combined = sql.match(matchPatterns.combined);
+		matches.combined = sql.match(matchPatterns.joinCombined);
 
 		// Flatten the matches to increase relevance
 		Object.keys(matches).forEach(function(key) {
@@ -83,7 +113,146 @@ var QueryParser = function(driver) {
 			}
 		});
 
-		return parts.combined.join('');
+		return parts.combined.join(' ');
+	};
+
+	/**
+	 * Parse a where clause to separate functions from values
+	 *
+	 * @param {Object} driver
+	 * @param {State} state
+	 * @return {String} - The parsed/escaped where condition
+	 */
+	this.parseWhere = function(driver, state) {
+		var whereMap = state.whereMap,
+			whereValues = state.rawWhereValues;
+
+		var outputMap = [];
+		var outputValues = [];
+		var that = this;
+
+		Object.keys(whereMap).forEach(function(key) {
+			// Combine fields, operators, functions and values into a full clause
+			// to have a common starting flow
+			var fullClause = '';
+
+			// Add an explicit = sign where one is inferred
+			if ( ! that.hasOperator(key))
+			{
+				fullClause = key + ' = ' + whereMap[key];
+			}
+			else if (whereMap[key] === key)
+			{
+				fullClause = key;
+			}
+			else
+			{
+				fullClause = key + ' ' + whereMap[key];
+			}
+
+			// Separate the clause into separate pieces
+			var parts = that.parseJoin(fullClause);
+
+			// Filter explicit literals from lists of matches
+			if (whereValues.indexOf(whereMap[key]) !== -1)
+			{
+				var value = whereMap[key];
+				var identIndex = (helpers.isArray(parts.identifiers)) ? parts.identifiers.indexOf(value) : -1;
+				var litIndex = (helpers.isArray(parts.literals)) ? parts.literals.indexOf(value) : -1;
+				var combIndex = (helpers.isArray(parts.combined)) ? parts.combined.indexOf(value) : -1;
+				var funcIndex = (helpers.isArray(parts.functions)) ? parts.functions.indexOf(value) : -1;
+				var inOutputArray = outputValues.indexOf(value) !== -1;
+
+				// Remove the identifier in question,
+				// and add to the output values array
+				if (identIndex !== -1)
+				{
+					parts.identifiers.splice(identIndex, 1);
+
+					if ( ! inOutputArray)
+					{
+						outputValues.push(value);
+						inOutputArray = true;
+					}
+				}
+
+				// Remove the value from the literals list
+				// so it is not added twice
+				if (litIndex !== -1)
+				{
+					parts.literals.splice(litIndex, 1);
+
+					if ( ! inOutputArray)
+					{
+						outputValues.push(value);
+						inOutputArray = true;
+					}
+				}
+
+				// Remove the value from the combined list
+				// and replace it with a placeholder
+				if (combIndex !== -1)
+				{
+					// Make sure to skip functions when replacing values
+					if (funcIndex === -1)
+					{
+						parts.combined[combIndex] = '?';
+
+						if ( ! inOutputArray)
+						{
+							outputValues.push(value);
+							inOutputArray = true;
+						}
+					}
+				}
+			}
+
+			// Filter false positive identifiers
+			parts.identifiers = parts.identifiers.filter(function(item) {
+				var isInCombinedMatches = parts.combined.indexOf(item) !== -1;
+				var isNotInBlackList = identifierBlacklist.indexOf(item.toLowerCase()) === -1;
+
+				return isInCombinedMatches && isNotInBlackList;
+			});
+
+			// Quote identifiers
+			if (helpers.isArray(parts.identifiers))
+			{
+				parts.identifiers.forEach(function(ident) {
+					var index = parts.combined.indexOf(ident);
+					if (index !== -1)
+					{
+						parts.combined[index] = driver.quoteIdentifiers(ident);
+					}
+				});
+			}
+
+			// Replace each literal with a placeholder in the map
+			// and add the literal to the values,
+			// This should only apply to literal values that are not
+			// explicitly mapped to values, but have to be parsed from
+			// a where condition,
+			if (helpers.isArray(parts.literals))
+			{
+				parts.literals.forEach(function(lit) {
+					var litIndex = parts.combined.indexOf(lit);
+
+					if (litIndex !== -1)
+					{
+						parts.combined[litIndex] = (helpers.isArray(parts.operators)) ? '?' : '= ?';
+						outputValues.push(lit);
+					}
+				});
+			}
+
+			outputMap.push(parts.combined.join(' '));
+		});
+
+		state.rawWhereValues = [];
+		state.whereValues = state.whereValues.concat(outputValues);
+		state.whereMap = outputMap;
+
+		return state;
 	};
 };
 

lib/state.js

@@ -0,0 +1,29 @@
+'use strict';
+
+/** @module State */
+module.exports = function State() {
+	return {
+		// Arrays/Maps
+		queryMap: [],
+		values: [],
+		whereValues: [],
+		setArrayKeys: [],
+		orderArray: [],
+		groupArray: [],
+		havingMap: [],
+		whereMap: {},
+		rawWhereValues: [],
+
+		// Partials
+		selectString: '',
+		fromString: '',
+		setString: '',
+		orderString: '',
+		groupString: '',
+
+		// Other various values
+		limit: null,
+		offset: null
+	};
+};
+// End of module State

tests/adapters/dblite_test.js

@@ -54,6 +54,22 @@ if (connection)
 		test.done();
 	};
 
+	tests['Select tests']['Select with function and argument in WHERE clause'] = function(test) {
+		test.expect(1);
+		qb.select('id')
+			.from('create_test')
+			.where('id', 'ABS(-88)')
+			.get(function(err, rows) {
+				if (err != null) {
+					test.done();
+					throw new Error(err);
+				}
+
+				test.ok(rows, 'dblite: Valid result for generated query');
+				test.done();
+			});
+	};
+
 	tests["dblite adapter with query builder"] = function(test) {
 		test.expect(1);
 		test.ok(testBase.qb);

tests/adapters/mysql_test.js

@@ -39,9 +39,12 @@ tests['nodeQuery.getQuery = nodeQuery.init'] = function(test) {
 tests["mysql adapter with query builder"] = function(test) {
 	test.expect(1);
 	test.ok(testBase.qb);
-	test.done();
+
 	// Close the db connection
-	connection.end();
+	qb = null;
+	connection.destroy();
+
+	test.done();
 };
 
 // Export the final test object

tests/adapters/sqlite3_test.js

@@ -51,6 +51,22 @@ if (connection)
 		test.done();
 	});
 
+	tests['Select tests']['Select with function and argument in WHERE clause'] = function(test) {
+		test.expect(1);
+		qb.select('id')
+			.from('create_test')
+			.where('id', 'ABS(-88)')
+			.get(function(err, rows) {
+				if (err != null) {
+					test.done();
+					throw new Error(err);
+				}
+
+				test.ok(rows, 'sqlite3: Valid result for generated query');
+				test.done();
+			});
+	};
+
 	tests['nodeQuery.getQuery = nodeQuery.init'] = function(test) {
 		test.expect(1);
 		test.deepEqual(qb, nodeQuery.getQuery(), "getQuery returns same object");

tests/query-builder-base.js

@@ -1,6 +1,7 @@
 'use strict';
 
 var helpers = require('../lib/helpers');
+var State = require('../lib/state');
 
 module.exports = (function QueryBuilderTestBase()  {
 
@@ -143,6 +144,27 @@ module.exports = (function QueryBuilderTestBase()  {
 					.where('id', 3)
 					.orWhereIsNull('id')
 					.get(base.testCallback.bind(this, test));
+			},
+			'Select with string where value': function(test) {
+				test.expect(1);
+				base.qb.select('id','key as k', 'val')
+					.from('create_test ct')
+					.where('id > 3')
+					.get(base.testCallback.bind(this, test));
+			},
+			/*'Select with function in WHERE clause': function(test) {
+				test.expect(1);
+				base.qb.select('id', 'key as k', 'val')
+					.from('create_test')
+					.where('val !=', 'CURRENT_TIMESTAMP()')
+					.get(base.testCallback.bind(this, test));
+			},*/
+			'Select with function and argument in WHERE clause': function(test) {
+				test.expect(1);
+				base.qb.select('id')
+					.from('create_test')
+					.where('id', 'CEILING(SQRT(88))')
+					.get(base.testCallback.bind(this, test));
 			}
 		},
 		// ! Grouping tests
@@ -488,28 +510,7 @@ module.exports = (function QueryBuilderTestBase()  {
 					.from('bar')
 					.where('baz', 'foobar');
 
-				var state = {
+				var state = new State();
-					// Arrays/Maps
-					queryMap: [],
-					values: [],
-					whereValues: [],
-					setArrayKeys: [],
-					orderArray: [],
-					groupArray: [],
-					havingMap: [],
-					whereMap: {},
-
-					// Partials
-					selectString: '',
-					fromString: '',
-					setString: '',
-					orderString: '',
-					groupString: '',
-
-					// Other various values
-					limit: null,
-					offset: null
-				};
 
 				test.notDeepEqual(JSON.stringify(state), JSON.stringify(base.qb.getState()));
 				test.done();
@@ -523,28 +524,7 @@ module.exports = (function QueryBuilderTestBase()  {
 
 				base.qb.resetQuery();
 
-				var state = {
+				var state = new State();
-					// Arrays/Maps
-					queryMap: [],
-					values: [],
-					whereValues: [],
-					setArrayKeys: [],
-					orderArray: [],
-					groupArray: [],
-					havingMap: [],
-					whereMap: {},
-
-					// Partials
-					selectString: '',
-					fromString: '',
-					setString: '',
-					orderString: '',
-					groupString: '',
-
-					// Other various values
-					limit: null,
-					offset: null
-				};
 
 				test.deepEqual(state, base.qb.getState());
 

tests/query-parser_test.js

@@ -2,10 +2,131 @@
 
 // Use the base driver as a mock for testing
 delete require.cache[require.resolve('../lib/driver')];
+var getArgs = require('getargs');
+var helpers = require('../lib/helpers');
 var driver = require('../lib/driver');
 var	parser = require('../lib/query-parser')(driver);
+var State = require('../lib/state');
+
+// Simulate query builder state
+var state = new State();
+
+var mixedSet = function(/* $varName, $valType, $key, [$val] */) {
+	var args = getArgs('$varName:string, $valType:string, $key:object|string|number, [$val]', arguments);
+
+	var obj = {};
+
+
+	if (helpers.isScalar(args.$key) && !helpers.isUndefined(args.$val))
+	{
+		// Convert key/val pair to a simple object
+		obj[args.$key] = args.$val;
+	}
+	else if (helpers.isScalar(args.$key) && helpers.isUndefined(args.$val))
+	{
+		// If just a string for the key, and no value, create a simple object with duplicate key/val
+		obj[args.$key] = args.$key;
+	}
+	else
+	{
+		obj = args.$key;
+	}
+
+	Object.keys(obj).forEach(function(k) {
+		// If a single value for the return
+		if (['key','value'].indexOf(args.$valType) !== -1)
+		{
+			var pushVal = (args.$valType === 'key') ? k : obj[k];
+			state[args.$varName].push(pushVal);
+		}
+		else
+		{
+			state[args.$varName][k] = obj[k];
+		}
+	});
+
+
+	return state[args.$varName];
+}
+
+var whereMock = function() {
+	var args = getArgs('key:string|object, [val]', arguments);
+
+	state.whereMap = [];
+	state.whereValues = [];
+
+	mixedSet('rawWhereValues', 'value', args.key, args.val);
+	mixedSet('whereMap', 'both', args.key, args.val);
+}
+
+// -----------------------------------------------------------------------------
+// ! Start Tests
+// -----------------------------------------------------------------------------
 
 module.exports = {
+	'Has operator tests': {
+		'Has operator': function(test) {
+			var matches = parser.hasOperator('foo <> 2');
+			test.deepEqual(['<>'], matches);
+			test.done();
+		},
+		'Has no operator': function(test) {
+			var matches = parser.hasOperator('foo');
+			test.equal(null, matches);
+			test.done();
+		}
+	},
+	'Where parser tests': {
+		'Has function full string': function(test) {
+			test.expect(1);
+			whereMock('time < SUM(FOO(BAR()))');
+			var result = parser.parseWhere(driver, state);
+			test.deepEqual(['"time" < SUM(FOO(BAR()))'], state.whereMap);
+
+			test.done();
+		},
+		'Has function key/val': function(test) {
+			test.expect(1);
+			var map = whereMock('time <', 'SUM(FOO(BAR()))');
+			state = parser.parseWhere(driver, state);
+			test.deepEqual(['"time" < SUM(FOO(BAR()))'], state.whereMap);
+
+			test.done();
+		},
+		'Has function key/val object': function(test) {
+			test.expect(1);
+			var map = whereMock({
+				'time <': "SUM(FOO(BAR('x')))"
+			});
+			state = parser.parseWhere(driver, state);
+			test.deepEqual(['"time" < SUM(FOO(BAR(\'x\')))'], state.whereMap);
+
+			test.done();
+		},
+		'Has literal value': function(test) {
+			test.expect(2);
+			var map = whereMock({
+				'foo': 3
+			});
+			state = parser.parseWhere(driver, state);
+			test.deepEqual(['"foo" = ?'], state.whereMap);
+			test.deepEqual(['3'], state.whereValues);
+
+			test.done();
+		},
+		'Has multiple literal values': function(test) {
+			test.expect(2);
+			var map = whereMock({
+				foo: 3,
+				bar: 5
+			});
+			state = parser.parseWhere(driver, state);
+			test.deepEqual(['"foo" = ?', '"bar" = ?'], state.whereMap);
+			test.deepEqual(['3','5'], state.whereValues);
+
+			test.done();
+		}
+	},
 	'Parse join tests' : {
 		'Simple equals condition': function(test) {
 			var matches = parser.parseJoin('table1.field1=table2.field2');
@@ -31,22 +152,22 @@ module.exports = {
 	'Compile join tests': {
 		'Simple equals condition': function(test) {
 			var join = parser.compileJoin('table1.field1=table2.field2');
-			test.deepEqual('"table1"."field1"="table2"."field2"', join);
+			test.deepEqual('"table1"."field1" = "table2"."field2"', join);
 			test.done();
 		},
 		'Db.table.field condition': function(test) {
 			var join = parser.compileJoin('db1.table1.field1!=db2.table2.field2');
-			test.deepEqual('"db1"."table1"."field1"!="db2"."table2"."field2"', join);
+			test.deepEqual('"db1"."table1"."field1" != "db2"."table2"."field2"', join);
 			test.done();
 		},
 		'Underscore in identifier': function(test) {
 			var join = parser.compileJoin('table_1.field1 = tab_le2.field_2');
-			test.deepEqual('"table_1"."field1"="tab_le2"."field_2"', join);
+			test.deepEqual('"table_1"."field1" = "tab_le2"."field_2"', join);
 			test.done();
 		},
 		'Function in condition': function(test) {
 			var join = parser.compileJoin('table1.field1 > SUM(3+6)');
-			test.deepEqual('"table1"."field1">SUM(3+6)', join);
+			test.deepEqual('"table1"."field1" > SUM(3+6)', join);
 			test.done();
 		}
 	}