Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply boost only once for distance_feature query #63767

Merged
merged 2 commits into from
Oct 16, 2020
Merged

Apply boost only once for distance_feature query #63767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6654c68
Apply boost only once for distance_feature query
mayya-sharipova Oct 15, 2020
7cbe160
Remove test
mayya-sharipova Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we fix LongScriptFieldDistanceFeatureQuery, I think we could remove the boost parameter altogether from MappedFieldType#distanceFeatureQuery ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I or Nik will do that.

}

String fieldName() {
Expand Down
5 changes: 2 additions & 3 deletions server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down