Apply boost only once for distance_feature query

Currently if distance_feature query contains boost, it incorrectly gets applied twice: in AbstractQueryBuilder::toQuery and we also pass this boost to Lucene's LongPoint.newDistanceFeatureQuery. As a result we get incorrect scores. This fixes this error to ensure that boost is applied only once. Closes elastic#63691
mayya-sharipova · Oct 15, 2020 · 6654c68 · 6654c68
1 parent bf6dc58
commit 6654c68
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 4 deletions.
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java
@@ -45,6 +45,7 @@
 import org.elasticsearch.index.analysis.NormalizingCharFilterFactory;
 import org.elasticsearch.index.analysis.TokenizerFactory;
 import org.elasticsearch.index.query.BoolQueryBuilder;
+import org.elasticsearch.index.query.DistanceFeatureQueryBuilder;
 import org.elasticsearch.index.query.MatchQueryBuilder;
 import org.elasticsearch.index.query.MultiMatchQueryBuilder;
 import org.elasticsearch.index.query.Operator;
@@ -1626,6 +1627,33 @@ public void testRangeQueryWithLocaleMapping() throws Exception {
         assertHitCount(searchResponse, 2L);
     }
 
+    public void testDistanceFeatureQuery() {
+        assertAcked(prepareCreate("test").setMapping("date", "type=date"));
+        client().prepareIndex("test").setId("1").setSource("date", "2020-12-03T00:00:00Z").get();
+        client().prepareIndex("test").setId("2").setSource("date", "2020-12-02T00:00:00Z").get();
+        client().prepareIndex("test").setId("3").setSource("date", "2020-12-01T00:00:00Z").get();
+        refresh();
+
+        DistanceFeatureQueryBuilder dqb = QueryBuilders.distanceFeatureQuery(
+            "date", new DistanceFeatureQueryBuilder.Origin("2020-12-01T00:00:00Z"), "1d");
+        SearchResponse searchResponse = client().prepareSearch("test").setQuery(dqb).get();
+        assertHitCount(searchResponse, 3L);
+        SearchHit[] hits = searchResponse.getHits().getHits();
+        assertEquals(1.0f, hits[0].getScore(), 0.1f);
+        assertEquals(0.5f, hits[1].getScore(), 0.1f);
+        assertEquals(0.33f, hits[2].getScore(), 0.1f);
+
+        DistanceFeatureQueryBuilder dqb2 = QueryBuilders.distanceFeatureQuery(
+            "date", new DistanceFeatureQueryBuilder.Origin("2020-12-01T00:00:00Z"), "1d");
+        dqb2.boost(2.0f);
+        SearchResponse searchResponse2 = client().prepareSearch("test").setQuery(dqb2).get();
+        assertHitCount(searchResponse2, 3L);
+        SearchHit[] hits2 = searchResponse2.getHits().getHits();
+        assertEquals(2.0f, hits2[0].getScore(), 0.1f);
+        assertEquals(1.0f, hits2[1].getScore(), 0.1f);
+        assertEquals(0.66f, hits2[2].getScore(),0.1f);
+    }
+
     public void testSearchEmptyDoc() {
         assertAcked(prepareCreate("test").setSettings("{\"index.analysis.analyzer.default.type\":\"keyword\"}", XContentType.JSON));
         client().prepareIndex("test").setId("1").setSource("{}", XContentType.JSON).get();

diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java
@@ -112,7 +112,8 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
         if (fieldType == null) {
             return Queries.newMatchNoDocsQuery("Can't run [" + NAME + "] query on unmapped fields!");
         }
-        return fieldType.distanceFeatureQuery(origin.origin(), pivot, boost, context);
+        // As we already apply boost in AbstractQueryBuilder::toQuery, we always passing a boost of 1.0 to distanceFeatureQuery
+        return fieldType.distanceFeatureQuery(origin.origin(), pivot, 1.0f, context);
     }
 
     String fieldName() {

diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java
@@ -77,12 +77,11 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder,
         String fieldName = expectedFieldName(queryBuilder.fieldName());
         Object origin = queryBuilder.origin().origin();
         String pivot = queryBuilder.pivot();
-        float boost = queryBuilder.boost;
         final Query expectedQuery;
         if (fieldName.equals(GEO_POINT_FIELD_NAME)) {
             GeoPoint originGeoPoint = (origin instanceof GeoPoint)? (GeoPoint) origin : GeoUtils.parseFromString((String) origin);
             double pivotDouble = DistanceUnit.DEFAULT.parse(pivot, DistanceUnit.DEFAULT);
-            expectedQuery = LatLonPoint.newDistanceFeatureQuery(fieldName, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble);
+            expectedQuery = LatLonPoint.newDistanceFeatureQuery(fieldName, 1.0f, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble);
         } else { // if (fieldName.equals(DATE_FIELD_NAME))
             DateFieldType fieldType = (DateFieldType) context.getFieldType(fieldName);
             long originLong = fieldType.parseToLong(origin, true, null, null, context::nowInMillis);
@@ -93,7 +92,7 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder,
             } else { // NANOSECONDS
                 pivotLong = pivotVal.getNanos();
             }
-            expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, boost, originLong, pivotLong);
+            expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, 1.0f, originLong, pivotLong);
         }
         assertEquals(expectedQuery, query);
     }