From c20737f45883b79df65214a0ec955c288b92084d Mon Sep 17 00:00:00 2001
From: Julian Simioni <julian@simioni.org>
Date: Tue, 16 Oct 2018 16:28:31 -0400
Subject: [PATCH] fix(boundary.country): use boundary.country as filter

By definition, all boundary.country query matches will either be
identical, or not a match. Thus, it does not make sense to put the query
clause for boundary.country in the `must` section of the query.

In theory, because our queries would generally combine this `must`
clause with others, there shouldn't be any performance improvement (or
regression) from this change.

However, semantically, this clause fits better as a `filter`, and in the
case of a bug causing a degenerate query with the `boundary.country`
query clause as the only one under the `must` section, this would have a
big impact.
---
 query/autocomplete.js                            |  2 +-
 query/reverse.js                                 |  3 ++-
 query/search_original.js                         |  2 +-
 .../fixture/autocomplete_boundary_country.js     | 15 ++++++++-------
 .../fixture/search_boundary_country_original.js  | 16 ++++++++--------
 test/unit/query/reverse.js                       |  4 ++--
 6 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/query/autocomplete.js b/query/autocomplete.js
index f5b37982..b0a9039c 100644
--- a/query/autocomplete.js
+++ b/query/autocomplete.js
@@ -21,7 +21,6 @@ var query = new peliasQuery.layout.FilteredBooleanQuery();
 // mandatory matches
 query.score( views.phrase_first_tokens_only, 'must' );
 query.score( views.ngrams_last_token_only, 'must' );
-query.score( peliasQuery.view.boundary_country, 'must' );
 
 // address components
 query.score( peliasQuery.view.address('housenumber') );
@@ -49,6 +48,7 @@ query.score( peliasQuery.view.population( views.pop_subquery ) );
 query.filter( peliasQuery.view.sources );
 query.filter( peliasQuery.view.layers );
 query.filter( peliasQuery.view.boundary_rect );
+query.filter( peliasQuery.view.boundary_country );
 
 // --------------------------------
 
diff --git a/query/reverse.js b/query/reverse.js
index 1e928344..1da5fb46 100644
--- a/query/reverse.js
+++ b/query/reverse.js
@@ -10,7 +10,7 @@ const logger = require('pelias-logger').get('api');
 var query = new peliasQuery.layout.FilteredBooleanQuery();
 
 // mandatory matches
-query.score( peliasQuery.view.boundary_country, 'must' );
+// (none)
 
 // scoring boost
 query.sort( peliasQuery.view.sort_distance );
@@ -20,6 +20,7 @@ query.filter( peliasQuery.view.boundary_circle );
 query.filter( peliasQuery.view.sources );
 query.filter( peliasQuery.view.layers );
 query.filter( peliasQuery.view.categories );
+query.filter( peliasQuery.view.boundary_country );
 
 // --------------------------------
 
diff --git a/query/search_original.js b/query/search_original.js
index 6b9856b7..1631245e 100644
--- a/query/search_original.js
+++ b/query/search_original.js
@@ -18,7 +18,6 @@ var adminFields = placeTypes.concat(['region_a']);
 var query = new peliasQuery.layout.FilteredBooleanQuery();
 
 // mandatory matches
-query.score( peliasQuery.view.boundary_country, 'must' );
 query.score( peliasQuery.view.ngrams, 'must' );
 
 // scoring boost
@@ -46,6 +45,7 @@ query.filter( peliasQuery.view.boundary_rect );
 query.filter( peliasQuery.view.sources );
 query.filter( peliasQuery.view.layers );
 query.filter( peliasQuery.view.categories );
+query.filter( peliasQuery.view.boundary_country );
 
 // --------------------------------
 
diff --git a/test/unit/fixture/autocomplete_boundary_country.js b/test/unit/fixture/autocomplete_boundary_country.js
index 928c3efe..ef0c1666 100644
--- a/test/unit/fixture/autocomplete_boundary_country.js
+++ b/test/unit/fixture/autocomplete_boundary_country.js
@@ -16,13 +16,6 @@ module.exports = {
             }
           }
         }
-      }, {
-        'match': {
-          'parent.country_a': {
-            'analyzer': 'standard',
-            'query': 'ABC'
-          }
-        }
       }],
       'should':[{
         'function_score': {
@@ -58,6 +51,14 @@ module.exports = {
             'weight': 3
           }]
         }
+      }],
+      'filter': [{
+        'match': {
+          'parent.country_a': {
+            'analyzer': 'standard',
+            'query': 'ABC'
+          }
+        }
       }]
     }
   },
diff --git a/test/unit/fixture/search_boundary_country_original.js b/test/unit/fixture/search_boundary_country_original.js
index 0ea31ab3..aafcc3d5 100644
--- a/test/unit/fixture/search_boundary_country_original.js
+++ b/test/unit/fixture/search_boundary_country_original.js
@@ -2,14 +2,6 @@ module.exports = {
   'query': {
     'bool': {
       'must': [
-        {
-          'match': {
-            'parent.country_a': {
-              'analyzer': 'standard',
-              'query': 'ABC'
-            }
-          }
-        },
         {
           'match': {
             'name.default': {
@@ -88,6 +80,14 @@ module.exports = {
               'test'
             ]
           }
+        },
+        {
+          'match': {
+            'parent.country_a': {
+              'analyzer': 'standard',
+              'query': 'ABC'
+            }
+          }
         }
       ]
     }
diff --git a/test/unit/query/reverse.js b/test/unit/query/reverse.js
index 78c98e0c..6a213947 100644
--- a/test/unit/query/reverse.js
+++ b/test/unit/query/reverse.js
@@ -56,14 +56,14 @@ module.exports.tests.query = (test, common) => {
     t.notOk(query.body.vs.isset('input:categories'));
 
     t.deepEquals(query.body.score_functions, [
-      'boundary_country view'
     ]);
 
     t.deepEquals(query.body.filter_functions, [
       'boundary_circle view',
       'sources view',
       'layers view',
-      'categories view'
+      'categories view',
+      'boundary_country view'
     ]);
 
     t.deepEquals(query.body.sort_functions, [