diff --git a/lib/adapters/redis.js b/lib/adapters/redis.js index 4ecfac8d..bdd249f9 100644 --- a/lib/adapters/redis.js +++ b/lib/adapters/redis.js @@ -125,13 +125,16 @@ BridgeToRedis.prototype.possibleIndexes = function (model, filter) { if (!filter || Object.keys(filter.where || {}).length === 0) return false; var foundIndex = []; + var noIndex = []; Object.keys(filter.where).forEach(function (key) { - if (this.indexes[model][key] && typeof filter.where[key] === 'string') { + if (this.indexes[model][key] && (typeof filter.where[key] === 'string' || typeof filter.where[key] === 'number')) { foundIndex.push('i:' + model + ':' + key + ':' + filter.where[key]); + } else { + noIndex.push(key); } }.bind(this)); - return foundIndex; + return [foundIndex, noIndex]; }; BridgeToRedis.prototype.all = function all(model, filter, callback) { @@ -145,61 +148,34 @@ BridgeToRedis.prototype.all = function all(model, filter, callback) { var props = this._models[model].properties; var allNumeric = true; - // ORDER - if (filter && filter.order){ - var orders = filter.order; - if (typeof filter.order === "string"){ - orders = [filter.order]; - } - orders.forEach(function (key){ - if (props[key].type.name !== 'Number' && props[key].type.name !== 'Date') { - allNumeric = false; - } - sortCmd.push("BY", model + ":*->" + key); - }); - } - // LIMIT - if (filter && filter.limit){ - var from = (filter.offset || 0), to = from + filter.limit; - sortCmd.push("LIMIT", from, to); - } + // TODO: we need strict mode when filtration only possible when we have indexes + // WHERE + if (filter && filter.where) { + var pi = this.possibleIndexes(model, filter); + var indexes = pi[0]; + var noIndexes = pi[1]; - // we need ALPHA modifier when sorting string values - // the only case it's not required - we sort numbers - // TODO: check if we sort numbers - if (!allNumeric) { - sortCmd.push('ALPHA'); - } - - // do we need to sort or to query normally - if (sortCmd.length) { - sortCmd.unshift("s:" + model); - sortCmd.push("GET", "#"); - cmd = "SORT " + sortCmd.join(" "); - sortCmd.push(function(err, ids){ - if (err) { - return callback(err, []); - } - var keys = ids.map(function (i) { - return model + ":" + i; - }); - handleKeys(err, keys); - }); - client.sort.apply(client, sortCmd); - } else { - // Do a normal key lookup with possbible indexes - var indexes = this.possibleIndexes(model, filter); if (indexes.length) { cmd = 'SINTER "' + indexes.join('" "') + '"'; - indexes.push(handleKeys); - client.sinter.apply(client, indexes, handleKeys); + if (noIndexes.length) { + log(model + ': no indexes found for ', noIndexes.join(', '), + 'slow sorting and filtering'); + } + indexes.push(noIndexes.length ? orderLimitStageBad : orderLimitStage); + client.sinter.apply(client, indexes); } else { + // filter manually cmd = 'KEYS ' + model + ':*'; - client.keys(model + ':*', handleKeys); + client.keys(model + ':*', orderLimitStageBad); } + } else { + // no filtering, just sort/limit (if any) + gotKeys('*'); } - function handleKeys(err, keys) { + // bad case when we trying to filter on non-indexed fields + // in bad case we need retrieve all data and filter/limit/sort manually + function orderLimitStageBad(err, keys) { log(cmd, t1); var t2 = Date.now(); if (err) { @@ -208,12 +184,141 @@ BridgeToRedis.prototype.all = function all(model, filter, callback) { var query = keys.map(function (key) { return ['hgetall', key]; }); + client.multi(query).exec(function (err, replies) { + log(query, t2); + gotFilteredData(err, replies.filter(applyFilter(filter))); + }); + + function gotFilteredData(err, nodes) { + if (err) return callback(null); + + if (filter.order) { + var allNumeric = true; + var orders = filter.order; + if (typeof filter.order === "string") { + orders = [filter.order]; + } + orders.forEach(function (key) { + if (props[key].type.name !== 'Number' && props[key].type.name !== 'Date') { + allNumeric = false; + } + }); + if (allNumeric) { + nodes = nodes.sort(numerically.bind(orders)); + } else { + nodes = nodes.sort(literally.bind(orders)); + } + } + + // LIMIT + if (filter && filter.limit) { + var from = (filter.offset || 0), to = from + filter.limit; + callback(null, nodes.slice(from, to)); + } else { + callback(null, nodes); + } + } + } + + function orderLimitStage(err, keys) { + log(cmd, t1); + var t2 = Date.now(); + if (err) { + return callback(err, []); + } + + gotKeys(keys); + } + + function gotKeys(keys) { + + // ORDER + if (filter && filter.order) { + var orders = filter.order; + if (typeof filter.order === "string"){ + orders = [filter.order]; + } + orders.forEach(function (key) { + if (props[key].type.name !== 'Number' && props[key].type.name !== 'Date') { + allNumeric = false; + } + sortCmd.push("BY", model + ":*->" + key); + }); + } + + // LIMIT + if (keys === '*' && filter && filter.limit){ + var from = (filter.offset || 0), to = from + filter.limit; + sortCmd.push("LIMIT", from, to); + } + + // we need ALPHA modifier when sorting string values + // the only case it's not required - we sort numbers + // TODO: check if we sort numbers + if (!allNumeric) { + sortCmd.push('ALPHA'); + } + + if (sortCmd.length) { + sortCmd.unshift("s:" + model); + sortCmd.push("GET", "#"); + cmd = "SORT " + sortCmd.join(" "); + var ttt = Date.now(); + sortCmd.push(function(err, ids){ + if (err) { + return callback(err, []); + } + log(cmd, ttt); + var sortedKeys = ids.map(function (i) { + return model + ":" + i; + }); + handleKeys(err, intersect(sortedKeys, keys)); + }); + client.sort.apply(client, sortCmd); + } else { + // no sorting or filtering: just get all keys + if (keys === '*') { + cmd = 'KEYS ' + model + ':*'; + client.keys(model + ':*', handleKeys); + } else { + handleKeys(null, keys); + } + } + } + + function handleKeys(err, keys) { + var t2 = Date.now(); + var query = keys.map(function (key) { + return ['hgetall', key]; + }); client.multi(query).exec(function (err, replies) { log(query, t2); // console.log('Redis time: %dms', Date.now() - ts); callback(err, filter ? replies.filter(applyFilter(filter)) : replies); }); } + + return; + + function numerically(a, b) { + return a[this[0]] - b[this[0]]; + } + + function literally(a, b) { + return a[this[0]] > b[this[0]]; + } + + // TODO: find better intersection method + function intersect(sortedKeys, filteredKeys) { + if (filteredKeys === '*') return sortedKeys; + var index = {}; + filteredKeys.forEach(function (x) { + index[x] = true; + }); + return sortedKeys.filter(function (x) { + return index[x]; + }); + } }; function applyFilter(filter) { diff --git a/test/common_test.js b/test/common_test.js index a9da722a..95b0db51 100644 --- a/test/common_test.js +++ b/test/common_test.js @@ -59,7 +59,7 @@ function testOrm(schema) { Post = schema.define('Post', { title: { type: String, length: 255, index: true }, content: { type: Text }, - date: { type: Date, default: Date.now }, + date: { type: Date, default: Date.now, index: true }, published: { type: Boolean, default: false } }); @@ -424,16 +424,17 @@ function testOrm(schema) { }); it('should handle ORDER clause', function (test) { - var titles = [ 'Title A', 'Title Z', 'Title M', 'Title B' ]; - var dates = [ 5, 9, 0, 17 ]; + var titles = [ 'Title A', 'Title Z', 'Title M', 'Title B', 'Title C' ]; + var dates = [ 5, 9, 0, 17, 9 ]; titles.forEach(function (t, i) { Post.create({title: t, date: dates[i]}, done); }); - var i = 0; + var i = 0, tests = 0; function done(err, obj) { if (++i === titles.length) { doStringTest(); + doFilterAndSortTest(); doNumberTest(); } } @@ -441,6 +442,7 @@ function testOrm(schema) { // Post.schema.log = console.log; function doStringTest() { + tests += 1; Post.all({order: 'title'}, function (err, posts) { titles.sort().forEach(function (t, i) { test.equal(posts[i].title, t); @@ -450,6 +452,7 @@ function testOrm(schema) { } function doNumberTest() { + tests += 1; Post.all({order: 'date'}, function (err, posts) { dates.sort(numerically).forEach(function (d, i) { test.equal(posts[i].date, d); @@ -458,9 +461,22 @@ function testOrm(schema) { }); } + function doFilterAndSortTest() { + tests += 1; + Post.all({where: {date: 9}, order: 'title', limit: 3}, function (err, posts) { + test.equal(posts.length, 2, 'Exactly 2 posts returned by query'); + [ 'Title C', 'Title Z' ].forEach(function (t, i) { + if (posts[i]) { + test.equal(posts[i].title, t); + } + }); + finished(); + }); + } + var fin = 0; function finished() { - if (++fin === 2) { + if (++fin === tests) { test.done(); } }