From 91abc11f292027cb2541c289aa110e93d281ad4e Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Thu, 29 Jan 2015 17:59:01 -0500 Subject: [PATCH 01/11] adding a line break for readability --- query/search.js | 1 + 1 file changed, 1 insertion(+) diff --git a/query/search.js b/query/search.js index 4ccc9f6d..524db577 100644 --- a/query/search.js +++ b/query/search.js @@ -14,6 +14,7 @@ function generate( params ){ } var query = queries.distance( centroid, { size: params.size } ); + if (params.bbox) { query = queries.bbox ( centroid, { size: params.size, bbox: params.bbox } ); } From 2d7b11b967bfc00be220bdca7e23f20e6767d4a7 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Fri, 6 Feb 2015 14:06:10 -0500 Subject: [PATCH 02/11] using sort-scoring-function --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ffaa5841..73742830 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "express": "^4.8.8", "geojson": "^0.2.0", "geojson-extent": "^0.3.1", - "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#centroid-optional", + "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#sort-scoring-function", "is-object": "^1.0.1", "pelias-esclient": "0.0.25", "toobusy": "^0.2.4" From 5d7edd00e8628f65e65c22dfac71f6b4ca5c166e Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Fri, 6 Feb 2015 14:07:01 -0500 Subject: [PATCH 03/11] tests. adding sort to queries (although this should probably be decoupled from the api or the sort logic should be moved from elasticsearchbackend into api) --- test/unit/query/reverse.js | 43 +++++++++++++++++++++++++-- test/unit/query/search.js | 59 ++++++++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index 85a0720c..b04a3368 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -10,6 +10,42 @@ module.exports.tests.interface = function(test, common) { }); }; +var sort = [ + '_score', + { + '_script': { + 'script': 'if (doc.containsKey(\'population\')) { return doc[\'population\'].value } else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'geoname': 0, + 'address': 4, + 'osmnode': 6, + 'osmway': 6, + 'poi-address': 8, + 'neighborhood': 10, + 'local_admin': 12, + 'locality': 12, + 'admin2': 12, + 'admin1': 14, + 'admin0': 2 + } + }, + 'script': 'if (doc.containsKey(\'_type\')) { '+ + 'type=doc[\'_type\'].value; '+ + 'return ( type in weights ) ? weights[ type ] : 0 }'+ + 'else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + } +]; + module.exports.tests.query = function(test, common) { test('valid query', function(t) { var query = generate({ @@ -42,7 +78,7 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': [ + 'sort': sort.concat([ { '_geo_distance': { 'center_point': { @@ -53,8 +89,9 @@ module.exports.tests.query = function(test, common) { 'unit': 'km' } } - ], - 'size': 1 + ]), + 'size': 1, + 'track_scores': true }; t.deepEqual(query, expected, 'valid reverse query'); diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 5138f486..f7042139 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -10,6 +10,42 @@ module.exports.tests.interface = function(test, common) { }); }; +var sort = [ + '_score', + { + '_script': { + 'script': 'if (doc.containsKey(\'population\')) { return doc[\'population\'].value } else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': { + 'geoname': 0, + 'address': 4, + 'osmnode': 6, + 'osmway': 6, + 'poi-address': 8, + 'neighborhood': 10, + 'local_admin': 12, + 'locality': 12, + 'admin2': 12, + 'admin1': 14, + 'admin0': 2 + } + }, + 'script': 'if (doc.containsKey(\'_type\')) { '+ + 'type=doc[\'_type\'].value; '+ + 'return ( type in weights ) ? weights[ type ] : 0 }'+ + 'else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + } +]; + module.exports.tests.query = function(test, common) { test('valid query', function(t) { var query = generate({ @@ -55,10 +91,11 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': [], - 'size': 10 + 'sort': sort, + 'size': 10, + 'track_scores': true }; - + t.deepEqual(query, expected, 'valid search query'); t.end(); }); @@ -106,8 +143,9 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': [], - 'size': 10 + 'sort': sort, + 'size': 10, + 'track_scores': true }; t.deepEqual(query, expected, 'valid search query'); @@ -139,7 +177,9 @@ module.exports.tests.query = function(test, common) { } } }, - 'size': 10 + 'size': 10, + 'sort': sort, + 'track_scores': true }; t.deepEqual(query, expected, 'valid search query'); @@ -185,7 +225,7 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': [ + 'sort': sort.concat([ { '_geo_distance': { 'center_point': { @@ -196,8 +236,9 @@ module.exports.tests.query = function(test, common) { 'unit': 'km' } } - ], - 'size': 10 + ]), + 'size': 10, + 'track_scores': true }; t.deepEqual(query, expected, 'valid search query'); From 1a740f16df7cd882ade2b27f0f7b4d54db3aa444 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Sun, 8 Feb 2015 15:41:29 -0500 Subject: [PATCH 04/11] sorting function based on population and weights plus tests --- package.json | 1 + query/reverse.js | 8 ++++++-- query/search.js | 5 ++++- query/sort.js | 27 +++++++++++++++++++++++++++ test/unit/query/reverse.js | 6 +++--- test/unit/query/search.js | 5 +++-- 6 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 query/sort.js diff --git a/package.json b/package.json index 73742830..f54aa0e8 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "geojson": "^0.2.0", "geojson-extent": "^0.3.1", "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#sort-scoring-function", + "pelias-model": "git://github.com/pelias/model#multiplier", "is-object": "^1.0.1", "pelias-esclient": "0.0.25", "toobusy": "^0.2.4" diff --git a/query/reverse.js b/query/reverse.js index 3ada51be..35b86ade 100644 --- a/query/reverse.js +++ b/query/reverse.js @@ -1,6 +1,7 @@ var logger = require('../src/logger'), - queries = require('geopipes-elasticsearch-backend').queries; + queries = require('geopipes-elasticsearch-backend').queries, + sort = require('../query/sort'); function generate( params ){ @@ -9,7 +10,10 @@ function generate( params ){ lon: params.lon }; - return queries.distance( centroid, { size: params.size || 1 } ); + var query = queries.distance( centroid, { size: params.size || 1 } ); + query.sort = query.sort.concat(sort); + + return query; } module.exports = generate; \ No newline at end of file diff --git a/query/search.js b/query/search.js index 524db577..9d9916d3 100644 --- a/query/search.js +++ b/query/search.js @@ -1,6 +1,7 @@ var logger = require('../src/logger'), - queries = require('geopipes-elasticsearch-backend').queries; + queries = require('geopipes-elasticsearch-backend').queries, + sort = require('../query/sort'); function generate( params ){ @@ -28,6 +29,8 @@ function generate( params ){ } }; + query.sort = query.sort.concat(sort); + return query; } diff --git a/query/sort.js b/query/sort.js new file mode 100644 index 00000000..90eb265b --- /dev/null +++ b/query/sort.js @@ -0,0 +1,27 @@ +var population = 'population'; +var weights = require('pelias-model').weights; + +module.exports = [ + { + '_script': { + 'script': 'if (doc.containsKey(\''+ population + '\'))' + + ' { return doc[\'' + population + '\'].value }' + + ' else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': weights + }, + 'script': 'if (doc.containsKey(\'_type\')) { '+ + 'type=doc[\'_type\'].value; '+ + 'return ( type in weights ) ? weights[ type ] : 0 }' + + 'else { return 0 }', + 'type': 'number', + 'order': 'desc' + } + } +]; \ No newline at end of file diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index b04a3368..fb423895 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -11,7 +11,6 @@ module.exports.tests.interface = function(test, common) { }; var sort = [ - '_score', { '_script': { 'script': 'if (doc.containsKey(\'population\')) { return doc[\'population\'].value } else { return 0 }', @@ -78,7 +77,8 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': sort.concat([ + 'sort': [ + '_score', { '_geo_distance': { 'center_point': { @@ -89,7 +89,7 @@ module.exports.tests.query = function(test, common) { 'unit': 'km' } } - ]), + ].concat(sort), 'size': 1, 'track_scores': true }; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index f7042139..dc1f504e 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -225,7 +225,8 @@ module.exports.tests.query = function(test, common) { } } }, - 'sort': sort.concat([ + 'sort': [ + '_score', { '_geo_distance': { 'center_point': { @@ -236,7 +237,7 @@ module.exports.tests.query = function(test, common) { 'unit': 'km' } } - ]), + ].concat(sort.slice(1)), 'size': 10, 'track_scores': true }; From b4f4daafcf1dfc3dfcf0a00a368cc150a66c8956 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Mon, 9 Feb 2015 15:02:45 -0500 Subject: [PATCH 05/11] using suggester-pipeline instead of pelias-model --- package.json | 2 +- query/sort.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index f54aa0e8..cd0a3127 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "geojson": "^0.2.0", "geojson-extent": "^0.3.1", "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#sort-scoring-function", - "pelias-model": "git://github.com/pelias/model#multiplier", + "pelias-suggester-pipeline": "git://github.com/pelias/suggester-pipeline#using-model-weights", "is-object": "^1.0.1", "pelias-esclient": "0.0.25", "toobusy": "^0.2.4" diff --git a/query/sort.js b/query/sort.js index 90eb265b..7da2db5f 100644 --- a/query/sort.js +++ b/query/sort.js @@ -1,5 +1,5 @@ var population = 'population'; -var weights = require('pelias-model').weights; +var weights = require('pelias-suggester-pipeline').weights; module.exports = [ { From ff9e9973fcdbd40b5b1cbec7d54be4e4d8422300 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 10 Feb 2015 14:52:15 -0500 Subject: [PATCH 06/11] switching searchType from the default 'query_then_fetch' to 'dfs_query_then_fetch' because we have multiple shards in prod http://www.elasticsearch.org/blog/understanding-query-then-fetch-vs-dfs-query-then-fetch/ --- controller/search.js | 1 + test/unit/controller/search.js | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/controller/search.js b/controller/search.js index 36f6c3f1..aa644e47 100644 --- a/controller/search.js +++ b/controller/search.js @@ -13,6 +13,7 @@ function setup( backend, query ){ // backend command var cmd = { index: 'pelias', + searchType: 'dfs_query_then_fetch', body: query( req.clean ) }; diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index 19482eb9..7fd53ccb 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -53,7 +53,7 @@ module.exports.tests.functional_success = function(test, common) { test('functional success', function(t) { var backend = mockBackend( 'client/search/ok/1', function( cmd ){ - t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias' }, 'correct backend command'); + t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command'); }); var controller = setup( backend, mockQuery() ); var res = { @@ -78,7 +78,7 @@ module.exports.tests.functional_success = function(test, common) { module.exports.tests.functional_failure = function(test, common) { test('functional failure', function(t) { var backend = mockBackend( 'client/search/fail/1', function( cmd ){ - t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias' }, 'correct backend command'); + t.deepEqual(cmd, { body: { a: 'b' }, index: 'pelias', searchType: 'dfs_query_then_fetch' }, 'correct backend command'); }); var controller = setup( backend, mockQuery() ); var next = function( message ){ From aa721b8749e2cb7b93019e35eae6e6cc14363273 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Wed, 11 Feb 2015 17:01:21 -0500 Subject: [PATCH 07/11] moving groovy scripts to a file on disk @ elasticsearch/config/scripts to avoid dynamic script loading look at http://www.elasticsearch.org/blog/running-groovy-scripts-without-dynamic-scripting/ and https://github.com/pelias/scripts --- query/sort.js | 9 ++------- test/unit/query/reverse.js | 10 ++++------ test/unit/query/search.js | 7 ++----- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/query/sort.js b/query/sort.js index 7da2db5f..8ddbdfc0 100644 --- a/query/sort.js +++ b/query/sort.js @@ -4,9 +4,7 @@ var weights = require('pelias-suggester-pipeline').weights; module.exports = [ { '_script': { - 'script': 'if (doc.containsKey(\''+ population + '\'))' + - ' { return doc[\'' + population + '\'].value }' + - ' else { return 0 }', + 'file': 'population', 'type': 'number', 'order': 'desc' } @@ -16,10 +14,7 @@ module.exports = [ 'params': { 'weights': weights }, - 'script': 'if (doc.containsKey(\'_type\')) { '+ - 'type=doc[\'_type\'].value; '+ - 'return ( type in weights ) ? weights[ type ] : 0 }' + - 'else { return 0 }', + 'file': 'weights', 'type': 'number', 'order': 'desc' } diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index fb423895..a8d6d327 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -11,9 +11,10 @@ module.exports.tests.interface = function(test, common) { }; var sort = [ + '_score', { '_script': { - 'script': 'if (doc.containsKey(\'population\')) { return doc[\'population\'].value } else { return 0 }', + 'file': 'population', 'type': 'number', 'order': 'desc' } @@ -35,10 +36,7 @@ var sort = [ 'admin0': 2 } }, - 'script': 'if (doc.containsKey(\'_type\')) { '+ - 'type=doc[\'_type\'].value; '+ - 'return ( type in weights ) ? weights[ type ] : 0 }'+ - 'else { return 0 }', + 'file': 'weights', 'type': 'number', 'order': 'desc' } @@ -89,7 +87,7 @@ module.exports.tests.query = function(test, common) { 'unit': 'km' } } - ].concat(sort), + ].concat(sort.slice(1)), 'size': 1, 'track_scores': true }; diff --git a/test/unit/query/search.js b/test/unit/query/search.js index dc1f504e..2d99d0ff 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -14,7 +14,7 @@ var sort = [ '_score', { '_script': { - 'script': 'if (doc.containsKey(\'population\')) { return doc[\'population\'].value } else { return 0 }', + 'file': 'population', 'type': 'number', 'order': 'desc' } @@ -36,10 +36,7 @@ var sort = [ 'admin0': 2 } }, - 'script': 'if (doc.containsKey(\'_type\')) { '+ - 'type=doc[\'_type\'].value; '+ - 'return ( type in weights ) ? weights[ type ] : 0 }'+ - 'else { return 0 }', + 'file': 'weights', 'type': 'number', 'order': 'desc' } From 39b9564d62a6022ddf4f52d3d34cb1394a103bd8 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Wed, 11 Feb 2015 17:03:34 -0500 Subject: [PATCH 08/11] updating package.json --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index cd0a3127..41bde4b6 100644 --- a/package.json +++ b/package.json @@ -36,8 +36,8 @@ "express": "^4.8.8", "geojson": "^0.2.0", "geojson-extent": "^0.3.1", - "geopipes-elasticsearch-backend": "git://github.com/geopipes/elasticsearch-backend#sort-scoring-function", - "pelias-suggester-pipeline": "git://github.com/pelias/suggester-pipeline#using-model-weights", + "geopipes-elasticsearch-backend": "0.0.11", + "pelias-suggester-pipeline": "2.0.2", "is-object": "^1.0.1", "pelias-esclient": "0.0.25", "toobusy": "^0.2.4" From 4e9b5502a298f408358608766265b363afb2e750 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 24 Feb 2015 17:20:21 -0500 Subject: [PATCH 09/11] using population var --- query/sort.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/sort.js b/query/sort.js index 8ddbdfc0..7102651f 100644 --- a/query/sort.js +++ b/query/sort.js @@ -4,7 +4,7 @@ var weights = require('pelias-suggester-pipeline').weights; module.exports = [ { '_script': { - 'file': 'population', + 'file': population, 'type': 'number', 'order': 'desc' } From 6c802dfa1da6811d8f1412a9bcf97db4c8360ff9 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 24 Feb 2015 18:48:22 -0500 Subject: [PATCH 10/11] testing sort part of the query --- test/unit/query/sort.js | 52 +++++++++++++++++++++++++++++++++++++++++ test/unit/run.js | 1 + 2 files changed, 53 insertions(+) create mode 100644 test/unit/query/sort.js diff --git a/test/unit/query/sort.js b/test/unit/query/sort.js new file mode 100644 index 00000000..ad5f9037 --- /dev/null +++ b/test/unit/query/sort.js @@ -0,0 +1,52 @@ + +var generate = require('../../../query/sort'); +var population = 'population'; +var weights = require('pelias-suggester-pipeline').weights; + +module.exports.tests = {}; + +module.exports.tests.interface = function(test, common) { + test('valid interface', function(t) { + t.equal(typeof generate, 'object', 'valid object'); + t.end(); + }); +}; + +var expected = [ + { + '_script': { + 'file': population, + 'type': 'number', + 'order': 'desc' + } + }, + { + '_script': { + 'params': { + 'weights': weights + }, + 'file': 'weights', + 'type': 'number', + 'order': 'desc' + } + } +]; + +module.exports.tests.query = function(test, common) { + test('valid part of query', function(t) { + var sort = generate; + t.deepEqual(sort, expected, 'valid sort part of the query'); + t.end(); + }); +}; + +module.exports.all = function (tape, common) { + + function test(name, testFunction) { + return tape('sort query ' + name, testFunction); + } + + for( var testCase in module.exports.tests ){ + module.exports.tests[testCase](test, common); + } +}; \ No newline at end of file diff --git a/test/unit/run.js b/test/unit/run.js index 20a5960d..4f5cffde 100644 --- a/test/unit/run.js +++ b/test/unit/run.js @@ -15,6 +15,7 @@ var tests = [ require('./sanitiser/coarse'), require('./query/indeces'), require('./query/suggest'), + require('./query/sort'), require('./query/search'), require('./query/reverse'), require('./helper/geojsonify'), From 743c825b41d44854055d8051248dcef3739879f5 Mon Sep 17 00:00:00 2001 From: Harish Krishna Date: Tue, 24 Feb 2015 18:48:56 -0500 Subject: [PATCH 11/11] just using weights from pelias-suggester-pipeline (reducing dependency issues with another repo) --- test/unit/query/reverse.js | 15 ++------------- test/unit/query/search.js | 15 ++------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js index a8d6d327..26c5e7e9 100644 --- a/test/unit/query/reverse.js +++ b/test/unit/query/reverse.js @@ -1,5 +1,6 @@ var generate = require('../../../query/reverse'); +var weights = require('pelias-suggester-pipeline').weights; module.exports.tests = {}; @@ -22,19 +23,7 @@ var sort = [ { '_script': { 'params': { - 'weights': { - 'geoname': 0, - 'address': 4, - 'osmnode': 6, - 'osmway': 6, - 'poi-address': 8, - 'neighborhood': 10, - 'local_admin': 12, - 'locality': 12, - 'admin2': 12, - 'admin1': 14, - 'admin0': 2 - } + 'weights': weights }, 'file': 'weights', 'type': 'number', diff --git a/test/unit/query/search.js b/test/unit/query/search.js index 2d99d0ff..3e9d72f0 100644 --- a/test/unit/query/search.js +++ b/test/unit/query/search.js @@ -1,5 +1,6 @@ var generate = require('../../../query/search'); +var weights = require('pelias-suggester-pipeline').weights; module.exports.tests = {}; @@ -22,19 +23,7 @@ var sort = [ { '_script': { 'params': { - 'weights': { - 'geoname': 0, - 'address': 4, - 'osmnode': 6, - 'osmway': 6, - 'poi-address': 8, - 'neighborhood': 10, - 'local_admin': 12, - 'locality': 12, - 'admin2': 12, - 'admin1': 14, - 'admin0': 2 - } + 'weights': weights }, 'file': 'weights', 'type': 'number',