From 7dd4447d13dfa2c211b2432e4d42febe19eac3e7 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Fri, 15 Feb 2019 12:17:57 -0500 Subject: [PATCH 1/7] Expose proximity boosting Expose DistanceFeatureQuery for long, geo and date types Closes #33382 --- .../query-dsl/distance-feature-query.asciidoc | 214 ++++++++++++++++++ .../query-dsl/special-queries.asciidoc | 8 + .../common/xcontent/ObjectParser.java | 1 + .../test/search/250_distance_feature.yml | 87 +++++++ .../elasticsearch/common/geo/GeoUtils.java | 10 + .../query/DistanceFeatureQueryBuilder.java | 190 ++++++++++++++++ .../elasticsearch/search/SearchModule.java | 3 + .../DistanceFeatureQueryBuilderTests.java | 208 +++++++++++++++++ .../search/SearchModuleTests.java | 3 +- .../test/AbstractBuilderTestCase.java | 6 +- 10 files changed, 727 insertions(+), 3 deletions(-) create mode 100644 docs/reference/query-dsl/distance-feature-query.asciidoc create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml create mode 100644 server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java create mode 100644 server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java diff --git a/docs/reference/query-dsl/distance-feature-query.asciidoc b/docs/reference/query-dsl/distance-feature-query.asciidoc new file mode 100644 index 0000000000000..9c4312d28e8e7 --- /dev/null +++ b/docs/reference/query-dsl/distance-feature-query.asciidoc @@ -0,0 +1,214 @@ +[[query-dsl-distance-feature-query]] +=== Distance Feature Query + +The `distance_feature` query is a specialized query that only works +on <>, <> or <> +fields. Its goal is to boost documents' scores based on proximity +to some given origin. For example, use this query if you want to +give more weight to documents with dates closer to a certain date, +or to documents with locations closer to a certain location, +or to documents with a long field closer to a certain number. + +This query is called `distance_feature` query, because it dynamically +calculates distances between the given origin and documents' field values, +and use these distances as features to boost the documents' scores. + +`distance_feature` query is typically put in a `should` clause of a +<> query so that its score is added to the score +of the query. + +Compared to using <> or other +ways to modify the score, this query has the benefit of being able to +efficiently skip non-competitive hits when +<> is not set to `true`. Speedups may be +spectacular. + +==== Syntax of distance_feature query + +`distance_feature` query has the following syntax: +[source,js] +-------------------------------------------------- +"distance_feature": { + "field": "my_field", + "origin": , + "pivot": , + "boost" : +} +-------------------------------------------------- +// NOTCONSOLE + +[horizontal] +`field`:: + Required parameter. Defines a name of the field on which to calculate + distances. Must be a field of type `long`, `date`, or `geo_point`, + and must be indexed and has <>. + +`origin`:: + Required parameter. Defines a point of origin used for calculating + distances. Must be a long number for numeric fields, date for date fields + and geo point for geo fields. Date math (for example `now-1h`) is + supported for a date origin. + +`pivot`:: + Required parameter. Defines the distance from origin at which the computed + score will equal to a half of the `boost` parameter. Must be a long + number for numeric fields, a `number+date unit` ("1h", "10d",...) for + date fields, and a `number + geo unit` ("1km", "12m",...) for geo fields. + +`boost`:: + Optional parameter with a default value of `1`. Defines the factor by which + to multiply the score. Must be a non-negative float number. + + +The `distance_feature` query computes a document's score as following: + +`score = boost * pivot / (pivot + distance)` + +where `distance` is the absolute difference between the origin and +a document's field value. For date field the distance will be in +milliseconds; for geo fields the distance is a haversine distance in meters. + +==== Example using distance_feature query + +Let's look at an example. We index several documents containing +information about sales items, such as name, production date, +price, and location. + +[source,js] +-------------------------------------------------- +PUT items +{ + "mappings": { + "properties": { + "item_name": { + "type": "keyword" + }, + "item_production_date": { + "type": "date" + }, + "item_price": { + "type": "long" + }, + "item_location": { + "type": "geo_point" + } + } + } +} + +PUT items/_doc/1 +{ + "item_name" : "chocolate", + "item_production_date": "2018-02-01", + "item_price": 22, + "item_location": [-71.34, 41.12] +} + +PUT items/_doc/2 +{ + "item_name" : "chocolate", + "item_production_date": "2018-01-01", + "item_price": 25, + "item_location": [-71.3, 41.15] +} + + +PUT items/_doc/3 +{ + "item_name" : "chocolate", + "item_expiry_date": "2017-12-01", + "item_production_date": 19, + "item_location": [-71.3, 41.12] +} + +POST items/_refresh +-------------------------------------------------- +// CONSOLE + +We look for all chocolate items, but we also want chocolates +that are closer to our origin price come first in the result list. + +[source,js] +-------------------------------------------------- +GET items/_search +{ + "query": { + "bool": { + "must": { + "match": { + "item_name": "chocolate" + } + }, + "should": { + "distance_feature": { + "boost" :2, + "field": "item_price", + "pivot": 5, + "origin": 15 + } + } + } + } +} +-------------------------------------------------- +// CONSOLE +// TEST[continued] + + +We look for all chocolate items, but we also want chocolates +that are produced recently (closer to the date `now`) +to be ranked higher. + +[source,js] +-------------------------------------------------- +GET items/_search +{ + "query": { + "bool": { + "must": { + "match": { + "item_name": "chocolate" + } + }, + "should": { + "distance_feature": { + "field": "item_production_date", + "pivot": "7d", + "origin": "now" + } + } + } + } +} +-------------------------------------------------- +// CONSOLE +// TEST[continued] + +We look for all chocolate items, but we also want chocolates +that are produced locally (closer to our geo origin) +come first in the result list. + +[source,js] +-------------------------------------------------- +GET items/_search +{ + "query": { + "bool": { + "must": { + "match": { + "item_name": "chocolate" + } + }, + "should": { + "distance_feature": { + "field": "item_location", + "pivot": "1000m", + "origin": [-71.3, 41.15] + } + } + } + } +} +-------------------------------------------------- +// CONSOLE +// TEST[continued] diff --git a/docs/reference/query-dsl/special-queries.asciidoc b/docs/reference/query-dsl/special-queries.asciidoc index 04ab2d53f6d35..9ac02cf6c981e 100644 --- a/docs/reference/query-dsl/special-queries.asciidoc +++ b/docs/reference/query-dsl/special-queries.asciidoc @@ -28,6 +28,12 @@ the specified document. A query that computes scores based on the values of numeric features and is able to efficiently skip non-competitive hits. +<>:: + +A query that computes scores based on the dynamically computed distances +between the origin and documents' long numeric, geo or distance fields. +It is able to efficiently skip non-competitive hits. + <>:: A query that accepts other queries as json or yaml string. @@ -42,4 +48,6 @@ include::percolate-query.asciidoc[] include::rank-feature-query.asciidoc[] +include::distance-feature-query.asciidoc[] + include::wrapper-query.asciidoc[] diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index dad79e0c0abad..ee5e3347f8d99 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -440,6 +440,7 @@ public enum ValueType { OBJECT_OR_LONG(START_OBJECT, VALUE_NUMBER), OBJECT_ARRAY_BOOLEAN_OR_STRING(START_OBJECT, START_ARRAY, VALUE_BOOLEAN, VALUE_STRING), OBJECT_ARRAY_OR_STRING(START_OBJECT, START_ARRAY, VALUE_STRING), + OBJECT_ARRAY_STRING_OR_NUMBER(START_OBJECT, START_ARRAY, VALUE_STRING, VALUE_NUMBER), VALUE(VALUE_BOOLEAN, VALUE_NULL, VALUE_EMBEDDED_OBJECT, VALUE_NUMBER, VALUE_STRING), VALUE_OBJECT_ARRAY(VALUE_BOOLEAN, VALUE_NULL, VALUE_EMBEDDED_OBJECT, VALUE_NUMBER, VALUE_STRING, START_OBJECT, START_ARRAY), VALUE_ARRAY(VALUE_BOOLEAN, VALUE_NULL, VALUE_NUMBER, VALUE_STRING, START_ARRAY); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml new file mode 100644 index 0000000000000..0fc4c0d682041 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml @@ -0,0 +1,87 @@ +setup: + - skip: + version: " - 7.9.99" #TODO adjust to 7.0.99 after merging to 7.x + reason: "Implemented in 7.1" + + - do: + indices.create: + index: index1 + body: + settings: + number_of_replicas: 0 + mappings: + properties: + my_long: + type: long + my_date: + type: date + my_geo: + type: geo_point + + - do: + bulk: + refresh: true + body: + - '{ "index" : { "_index" : "index1", "_id" : "1" } }' + - '{ "my_long" : 22, "my_date": "2018-02-01T10:00:30Z", "my_geo": [-71.34, 41.13] }' + - '{ "index" : { "_index" : "index1", "_id" : "2" } }' + - '{ "my_long" : 25, "my_date": "2018-02-01T11:00:30Z", "my_geo": [-71.34, 41.14] }' + - '{ "index" : { "_index" : "index1", "_id" : "3" } }' + - '{ "my_long" : 19, "my_date": "2018-02-01T09:00:30Z", "my_geo": [-71.34, 41.12] }' + +--- +"test distance_feature query on long type": + + - do: + search: + rest_total_hits_as_int: true + index: index1 + body: + query: + distance_feature: + field: my_long + pivot: 5 + origin: 15 + + - length: { hits.hits: 3 } + - match: { hits.hits.0._id: "3" } + - match: { hits.hits.1._id: "1" } + - match: { hits.hits.2._id: "2" } + +--- +"test distance_feature query on date type": + +- do: + search: + rest_total_hits_as_int: true + index: index1 + body: + query: + distance_feature: + field: my_date + pivot: 1h + origin: 2018-02-01T08:00:30Z + +- length: { hits.hits: 3 } +- match: { hits.hits.0._id: "3" } +- match: { hits.hits.1._id: "1" } +- match: { hits.hits.2._id: "2" } + +--- +"test distance_feature query on geo_point type": + +- do: + search: + rest_total_hits_as_int: true + index: index1 + body: + query: + distance_feature: + field: my_geo + pivot: 1km + origin: [-71.35, 41.12] + +- length: { hits.hits: 3 } +- match: { hits.hits.0._id: "3" } +- match: { hits.hits.1._id: "1" } +- match: { hits.hits.2._id: "2" } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 795cc235ce759..3ee49c3c6f72f 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java @@ -545,6 +545,16 @@ private static GeoPoint parseGeoHash(GeoPoint point, String geohash, EffectivePo } } + public static GeoPoint parseFromString(String val){ + GeoPoint point = new GeoPoint(); + boolean ignoreZValue = false; + if (val.contains(",")) { + return point.resetFromString(val, ignoreZValue); + } else { + return parseGeoHash(point, val, EffectivePoint.BOTTOM_LEFT); + } + } + /** * Parse a precision that can be expressed as an integer or a distance measure like "1km", "10m". * diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java new file mode 100644 index 0000000000000..d3509f60c6bf7 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -0,0 +1,190 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.document.LatLonPoint; +import org.apache.lucene.document.LongPoint; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeoUtils; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.unit.DistanceUnit; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.unit.TimeValue; + +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.GeoPointFieldMapper.GeoPointFieldType; +import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType; +import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType; +import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; + +import java.io.IOException; +import java.util.Objects; + +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; + +/** + * A query to boost scores based on their proximity to the given origin + */ +public class DistanceFeatureQueryBuilder extends AbstractQueryBuilder { + public static final String NAME = "distance_feature"; + + private static final ParseField FIELD_FIELD = new ParseField("field"); + private static final ParseField ORIGIN_FIELD = new ParseField("origin"); + private static final ParseField PIVOT_FIELD = new ParseField("pivot"); + + private final String field; + private final Object origin; //type of Long, String or GeoPoint + private final Object pivot; //type of Long or String + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "distance_feature", false, + args -> new DistanceFeatureQueryBuilder((String) args[0], args[1], args[2]) + ); + + static { + PARSER.declareString(constructorArg(), FIELD_FIELD); + // origin: number for long fields; number or string for date fields; string, array, object for geo fields + PARSER.declareField(constructorArg(), DistanceFeatureQueryBuilder::originFromXContent, + ORIGIN_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_STRING_OR_NUMBER); + //pivot: number for long fields; string for dates or geo fields + PARSER.declareField(constructorArg(), DistanceFeatureQueryBuilder::pivotFromXContent, + PIVOT_FIELD, ObjectParser.ValueType.LONG); + declareStandardFields(PARSER); + } + + public DistanceFeatureQueryBuilder(String field, Object origin, Object pivot) { + this.field = field; + this.origin = origin; + this.pivot = pivot; + } + + public static DistanceFeatureQueryBuilder fromXContent(XContentParser parser) { + try { + DistanceFeatureQueryBuilder builder = PARSER.apply(parser, null); + return builder; + } catch (IllegalArgumentException e) { + throw new ParsingException(parser.getTokenLocation(), e.getMessage(), e); + } + } + + public static Object originFromXContent(XContentParser parser) throws IOException { + if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { + return parser.longValue(); + } else if(parser.currentToken() == XContentParser.Token.VALUE_STRING) { + return parser.text(); + } else if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + return GeoUtils.parseGeoPoint(parser); + } else { // if currentToken equals to START_ARRAY) + return GeoUtils.parseGeoPoint(parser); + } + } + + public static Object pivotFromXContent(XContentParser parser) throws IOException { + if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { + return parser.longValue(); + } else { // if currentToken equals to VALUE_STRING + return parser.text(); + } + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(NAME); + builder.field(FIELD_FIELD.getPreferredName(), field); + builder.field(ORIGIN_FIELD.getPreferredName(), origin); + builder.field(PIVOT_FIELD.getPreferredName(), pivot); + printBoostAndQueryName(builder); + builder.endObject(); + } + + public DistanceFeatureQueryBuilder(StreamInput in) throws IOException { + super(in); + field = in.readString(); + origin = in.readGenericValue(); + pivot = in.readGenericValue(); + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + out.writeString(field); + out.writeGenericValue(origin); + out.writeGenericValue(pivot); + } + + @Override + public String getWriteableName() { + return NAME; + } + + @Override + protected Query doToQuery(QueryShardContext context) throws IOException { + MappedFieldType fieldType = context.fieldMapper(field); + if (fieldType == null) { + throw new IllegalArgumentException("Can't run [" + NAME + "] query on unmapped fields!"); + } + if (fieldType instanceof DateFieldType) { + long originLong = (origin instanceof Long) ? (Long) origin : + ((DateFieldType) fieldType).parseToLong(origin, true, null, null, context); + TimeValue val = TimeValue.parseTimeValue((String)pivot, TimeValue.timeValueHours(24), + DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); + long pivotLong = val.getMillis(); + return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotLong); + } else if ((fieldType instanceof NumberFieldType) && (fieldType.typeName() == NumberType.LONG.typeName())) { + long originLong = (Long) origin; + long pivotLong = (Long) pivot; + return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotLong); + } else if (fieldType instanceof GeoPointFieldType) { + GeoPoint originGeoPoint = (origin instanceof GeoPoint)? (GeoPoint) origin : GeoUtils.parseFromString((String) origin); + double pivotDouble = DistanceUnit.DEFAULT.parse((String) pivot, DistanceUnit.DEFAULT); + return LatLonPoint.newDistanceFeatureQuery(field, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); + } + throw new IllegalArgumentException( + "Illegal data type! ["+ NAME + "] query can only be run on a long, date or geo-point data type!"); + } + + public String fieldName() { + return field; + } + + public Object origin() { + return origin; + } + + public Object pivot() { + return pivot; + } + + @Override + protected int doHashCode() { + return Objects.hash(field, origin, pivot); + } + + @Override + protected boolean doEquals(DistanceFeatureQueryBuilder other) { + return this.field.equals(other.field) && Objects.equals(this.origin, other.origin) && Objects.equals(this.pivot, other.pivot); + } +} diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 81c6273ec1a36..e4963bb96af11 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -36,6 +36,7 @@ import org.elasticsearch.index.query.CommonTermsQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; import org.elasticsearch.index.query.DisMaxQueryBuilder; +import org.elasticsearch.index.query.DistanceFeatureQueryBuilder; import org.elasticsearch.index.query.ExistsQueryBuilder; import org.elasticsearch.index.query.FieldMaskingSpanQueryBuilder; import org.elasticsearch.index.query.FuzzyQueryBuilder; @@ -813,6 +814,8 @@ private void registerQueryParsers(List plugins) { registerQuery(new QuerySpec<>(MatchNoneQueryBuilder.NAME, MatchNoneQueryBuilder::new, MatchNoneQueryBuilder::fromXContent)); registerQuery(new QuerySpec<>(TermsSetQueryBuilder.NAME, TermsSetQueryBuilder::new, TermsSetQueryBuilder::fromXContent)); registerQuery(new QuerySpec<>(IntervalQueryBuilder.NAME, IntervalQueryBuilder::new, IntervalQueryBuilder::fromXContent)); + registerQuery(new QuerySpec<>(DistanceFeatureQueryBuilder.NAME, DistanceFeatureQueryBuilder::new, + DistanceFeatureQueryBuilder::fromXContent)); if (ShapesAvailability.JTS_AVAILABLE && ShapesAvailability.SPATIAL4J_AVAILABLE) { registerQuery(new QuerySpec<>(GeoShapeQueryBuilder.NAME, GeoShapeQueryBuilder::new, GeoShapeQueryBuilder::fromXContent)); diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java new file mode 100644 index 0000000000000..b142b2a59d5ed --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -0,0 +1,208 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.document.LatLonPoint; +import org.apache.lucene.document.LongPoint; +import org.apache.lucene.search.Query; +import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeoUtils; +import org.elasticsearch.common.unit.DistanceUnit; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.test.AbstractQueryTestCase; +import org.joda.time.DateTime; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; + +public class DistanceFeatureQueryBuilderTests extends AbstractQueryTestCase { + @Override + protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { + String field = randomFrom(LONG_FIELD_NAME, DATE_FIELD_NAME, GEO_POINT_FIELD_NAME); + Object origin; + Object pivot; + switch (field) { + case GEO_POINT_FIELD_NAME: + origin = new GeoPoint(randomDouble(), randomDouble()); + origin = randomBoolean()? origin : ((GeoPoint) origin).geohash(); + pivot = randomFrom(DistanceUnit.values()).toString(randomDouble()); + break; + case DATE_FIELD_NAME: + long randomDateMills = System.currentTimeMillis() - randomIntBetween(0, 1000000); + origin = randomBoolean() ? randomDateMills : new DateTime(randomDateMills).toString(); + pivot = randomTimeValue(1, 1000, "d", "h", "ms", "s", "m"); + break; + default: // LONG_FIELD_NAME + origin = randomLongBetween(1, Long.MAX_VALUE); + pivot = randomLongBetween(1, Long.MAX_VALUE); + break; + } + return new DistanceFeatureQueryBuilder(field, origin, pivot); + } + + @Override + protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { + String fieldName = expectedFieldName(queryBuilder.fieldName()); + Object origin = queryBuilder.origin(); + Object 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((String) pivot, DistanceUnit.DEFAULT); + expectedQuery = LatLonPoint.newDistanceFeatureQuery(fieldName, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); + } else if (fieldName.equals(DATE_FIELD_NAME)) { + MapperService mapperService = context.getQueryShardContext().getMapperService(); + MappedFieldType fieldType = mapperService.fullName(fieldName); + long originLong = (origin instanceof Long) ? (Long) origin : + ((DateFieldMapper.DateFieldType) fieldType).parseToLong(origin, true, null, null, context.getQueryShardContext()); + TimeValue val = TimeValue.parseTimeValue((String)pivot, TimeValue.timeValueHours(24), + DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); + long pivotLong = val.getMillis(); + expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, boost, originLong, pivotLong); + } else { + long originLong = (Long) origin; + long pivotLong = (Long) pivot; + expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, boost, originLong, pivotLong); + } + assertEquals(expectedQuery, query); + } + + public void testFromJsonLongFieldType() throws IOException { + String json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ LONG_FIELD_NAME + "\",\n" + + " \"origin\": 40,\n" + + " \"pivot\" : 10,\n" + + " \"boost\" : 2.0\n" + + " }\n" + + "}"; + DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + assertEquals(json, 40L, parsed.origin()); + assertEquals(json, 10L, parsed.pivot()); + assertEquals(json, 2.0, parsed.boost(), 0.0001); + } + + public void testFromJsonDateFieldType() throws IOException { + // origin as string + String origin = "2018-01-01T13:10:30Z"; + String pivot = "7d"; + String json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"origin\": \"" + origin + "\",\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + assertEquals(json, origin, parsed.origin()); + assertEquals(json, pivot, parsed.pivot()); + assertEquals(json, 1.0, parsed.boost(), 0.0001); + + // origin as long + long originLong = 1514812230999L; + json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"origin\": " + originLong + ",\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + assertEquals(json, originLong, parsed.origin()); + } + + public void testFromJsonGeoFieldType() throws IOException { + final GeoPoint origin = new GeoPoint(41.12,-71.34); + final String pivot = "1km"; + + // origin as string + String json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ GEO_POINT_FIELD_NAME + "\",\n" + + " \"origin\": \"" + origin.toString() + "\",\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 2.0\n" + + " }\n" + + "}"; + DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + checkGeneratedJson(json, parsed); + assertEquals(json, origin.toString(), parsed.origin()); + assertEquals(json, pivot, parsed.pivot()); + assertEquals(json, 2.0, parsed.boost(), 0.0001); + + // origin as array + json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ GEO_POINT_FIELD_NAME + "\",\n" + + " \"origin\": [" + origin.lon() + ", " + origin.lat() + "],\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 2.0\n" + + " }\n" + + "}"; + parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + assertEquals(json, origin, parsed.origin()); + + // origin as object + json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ GEO_POINT_FIELD_NAME + "\",\n" + + " \"origin\": {" + "\"lat\":"+ origin.lat() + ", \"lon\":"+ origin.lon() + "},\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 2.0\n" + + " }\n" + + "}"; + parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + assertEquals(json, origin, parsed.origin()); + } + + public void testQueryFailsWithUnmappedField() { + String query = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \"random_unmapped_field\",\n" + + " \"origin\": 40,\n" + + " \"pivot\" : 10\n" + + " }\n" + + "}"; + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(query).toQuery(createShardContext())); + assertEquals("Can't run [" + DistanceFeatureQueryBuilder.NAME + "] query on unmapped fields!", e.getMessage()); + } + + public void testQueryFailsWithWrongFieldType() { + String query = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ INT_FIELD_NAME + "\",\n" + + " \"origin\": 40,\n" + + " \"pivot\" : 10\n" + + " }\n" + + "}"; + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(query).toQuery(createShardContext())); + assertThat(e.getMessage(), containsString("query can only be run on a long, date or geo-point data type!")); + } +} diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java index f2d250fa1f80f..ccdfe28b44fdc 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java @@ -358,7 +358,8 @@ public List> getRescorers() { "terms_set", "type", "wildcard", - "wrapper" + "wrapper", + "distance_feature" }; //add here deprecated queries to make sure we log a deprecation warnings when they are used diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index 5eef0a249b687..cb28942658fb8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -104,6 +104,7 @@ public abstract class AbstractBuilderTestCase extends ESTestCase { protected static final String INT_FIELD_NAME = "mapped_int"; protected static final String INT_ALIAS_FIELD_NAME = "mapped_int_field_alias"; protected static final String INT_RANGE_FIELD_NAME = "mapped_int_range"; + protected static final String LONG_FIELD_NAME = "mapped_long"; protected static final String DOUBLE_FIELD_NAME = "mapped_double"; protected static final String BOOLEAN_FIELD_NAME = "mapped_boolean"; protected static final String DATE_FIELD_NAME = "mapped_date"; @@ -114,11 +115,11 @@ public abstract class AbstractBuilderTestCase extends ESTestCase { protected static final String GEO_POINT_ALIAS_FIELD_NAME = "mapped_geo_point_alias"; protected static final String GEO_SHAPE_FIELD_NAME = "mapped_geo_shape"; protected static final String[] MAPPED_FIELD_NAMES = new String[]{STRING_FIELD_NAME, STRING_ALIAS_FIELD_NAME, - INT_FIELD_NAME, INT_RANGE_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, LONG_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, DATE_RANGE_FIELD_NAME, OBJECT_FIELD_NAME, GEO_POINT_FIELD_NAME, GEO_POINT_ALIAS_FIELD_NAME, GEO_SHAPE_FIELD_NAME}; protected static final String[] MAPPED_LEAF_FIELD_NAMES = new String[]{STRING_FIELD_NAME, STRING_ALIAS_FIELD_NAME, - INT_FIELD_NAME, INT_RANGE_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, LONG_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, DATE_RANGE_FIELD_NAME, GEO_POINT_FIELD_NAME, GEO_POINT_ALIAS_FIELD_NAME}; private static final Map ALIAS_TO_CONCRETE_FIELD_NAME = new HashMap<>(); @@ -388,6 +389,7 @@ public void onRemoval(ShardId shardId, Accountable accountable) { INT_FIELD_NAME, "type=integer", INT_ALIAS_FIELD_NAME, "type=alias,path=" + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, "type=integer_range", + LONG_FIELD_NAME, "type=long", DOUBLE_FIELD_NAME, "type=double", BOOLEAN_FIELD_NAME, "type=boolean", DATE_FIELD_NAME, "type=date", From 0b2ae25d1da78826ee97f1b020d0ad14d4ab7d1d Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Thu, 7 Mar 2019 15:46:24 -0500 Subject: [PATCH 2/7] Address feedback: - Modify distance_feature query to be on date, date_nanos and geo fields only. Exclude long field - Make `origin` as an inner class --- .../query-dsl/distance-feature-query.asciidoc | 105 ++++--------- .../query-dsl/special-queries.asciidoc | 2 +- .../test/search/250_distance_feature.yml | 48 +++--- .../elasticsearch/common/geo/GeoUtils.java | 13 +- .../index/mapper/DateFieldMapper.java | 4 + .../query/DistanceFeatureQueryBuilder.java | 145 ++++++++++-------- .../DistanceFeatureQueryBuilderTests.java | 112 ++++++++------ .../index/query/TermsQueryBuilderTests.java | 3 +- .../test/AbstractBuilderTestCase.java | 8 +- .../test/AbstractQueryTestCase.java | 8 +- 10 files changed, 241 insertions(+), 207 deletions(-) diff --git a/docs/reference/query-dsl/distance-feature-query.asciidoc b/docs/reference/query-dsl/distance-feature-query.asciidoc index 9c4312d28e8e7..513449e04c627 100644 --- a/docs/reference/query-dsl/distance-feature-query.asciidoc +++ b/docs/reference/query-dsl/distance-feature-query.asciidoc @@ -2,26 +2,25 @@ === Distance Feature Query The `distance_feature` query is a specialized query that only works -on <>, <> or <> +on <>, <> or <> fields. Its goal is to boost documents' scores based on proximity to some given origin. For example, use this query if you want to give more weight to documents with dates closer to a certain date, -or to documents with locations closer to a certain location, -or to documents with a long field closer to a certain number. +or to documents with locations closer to a certain location. This query is called `distance_feature` query, because it dynamically calculates distances between the given origin and documents' field values, and use these distances as features to boost the documents' scores. -`distance_feature` query is typically put in a `should` clause of a +`distance_feature` query is typically used on its own to find the nearest +neighbors to a given point, or put in a `should` clause of a <> query so that its score is added to the score of the query. Compared to using <> or other ways to modify the score, this query has the benefit of being able to efficiently skip non-competitive hits when -<> is not set to `true`. Speedups may be -spectacular. +<> is not set to `true`. ==== Syntax of distance_feature query @@ -29,7 +28,7 @@ spectacular. [source,js] -------------------------------------------------- "distance_feature": { - "field": "my_field", + "field": , "origin": , "pivot": , "boost" : @@ -39,21 +38,22 @@ spectacular. [horizontal] `field`:: - Required parameter. Defines a name of the field on which to calculate - distances. Must be a field of type `long`, `date`, or `geo_point`, - and must be indexed and has <>. + Required parameter. Defines the name of the field on which to calculate + distances. Must be a field of the type `date`, `date_nanos` or `geo_point`, + and must be indexed (`"index": true`, which is the default) and has + <> (`"doc_values": true`, which is the default). `origin`:: Required parameter. Defines a point of origin used for calculating - distances. Must be a long number for numeric fields, date for date fields - and geo point for geo fields. Date math (for example `now-1h`) is + distances. Must be a date for date and date_nanos fields, + and a geo-point for geo_point fields. Date math (for example `now-1h`) is supported for a date origin. `pivot`:: Required parameter. Defines the distance from origin at which the computed - score will equal to a half of the `boost` parameter. Must be a long - number for numeric fields, a `number+date unit` ("1h", "10d",...) for - date fields, and a `number + geo unit` ("1km", "12m",...) for geo fields. + score will equal to a half of the `boost` parameter. Must be + a `number+date unit` ("1h", "10d",...) for date and date_nanos fields, + and a `number + geo unit` ("1km", "12m",...) for geo fields. `boost`:: Optional parameter with a default value of `1`. Defines the factor by which @@ -65,14 +65,13 @@ The `distance_feature` query computes a document's score as following: `score = boost * pivot / (pivot + distance)` where `distance` is the absolute difference between the origin and -a document's field value. For date field the distance will be in -milliseconds; for geo fields the distance is a haversine distance in meters. +a document's field value. ==== Example using distance_feature query Let's look at an example. We index several documents containing information about sales items, such as name, production date, -price, and location. +and location. [source,js] -------------------------------------------------- @@ -80,16 +79,13 @@ PUT items { "mappings": { "properties": { - "item_name": { + "name": { "type": "keyword" }, - "item_production_date": { + "production_date": { "type": "date" }, - "item_price": { - "type": "long" - }, - "item_location": { + "location": { "type": "geo_point" } } @@ -98,63 +94,30 @@ PUT items PUT items/_doc/1 { - "item_name" : "chocolate", - "item_production_date": "2018-02-01", - "item_price": 22, - "item_location": [-71.34, 41.12] + "name" : "chocolate", + "production_date": "2018-02-01", + "location": [-71.34, 41.12] } PUT items/_doc/2 { - "item_name" : "chocolate", - "item_production_date": "2018-01-01", - "item_price": 25, - "item_location": [-71.3, 41.15] + "name" : "chocolate", + "production_date": "2018-01-01", + "location": [-71.3, 41.15] } PUT items/_doc/3 { - "item_name" : "chocolate", - "item_expiry_date": "2017-12-01", - "item_production_date": 19, - "item_location": [-71.3, 41.12] + "name" : "chocolate", + "production_date": "2017-12-01", + "location": [-71.3, 41.12] } POST items/_refresh -------------------------------------------------- // CONSOLE -We look for all chocolate items, but we also want chocolates -that are closer to our origin price come first in the result list. - -[source,js] --------------------------------------------------- -GET items/_search -{ - "query": { - "bool": { - "must": { - "match": { - "item_name": "chocolate" - } - }, - "should": { - "distance_feature": { - "boost" :2, - "field": "item_price", - "pivot": 5, - "origin": 15 - } - } - } - } -} --------------------------------------------------- -// CONSOLE -// TEST[continued] - - We look for all chocolate items, but we also want chocolates that are produced recently (closer to the date `now`) to be ranked higher. @@ -167,12 +130,12 @@ GET items/_search "bool": { "must": { "match": { - "item_name": "chocolate" + "name": "chocolate" } }, "should": { "distance_feature": { - "field": "item_production_date", + "field": "production_date", "pivot": "7d", "origin": "now" } @@ -184,7 +147,7 @@ GET items/_search // CONSOLE // TEST[continued] -We look for all chocolate items, but we also want chocolates +We can look for all chocolate items, but we also want chocolates that are produced locally (closer to our geo origin) come first in the result list. @@ -196,12 +159,12 @@ GET items/_search "bool": { "must": { "match": { - "item_name": "chocolate" + "name": "chocolate" } }, "should": { "distance_feature": { - "field": "item_location", + "field": "location", "pivot": "1000m", "origin": [-71.3, 41.15] } diff --git a/docs/reference/query-dsl/special-queries.asciidoc b/docs/reference/query-dsl/special-queries.asciidoc index 9ac02cf6c981e..b7275ac2cee3f 100644 --- a/docs/reference/query-dsl/special-queries.asciidoc +++ b/docs/reference/query-dsl/special-queries.asciidoc @@ -31,7 +31,7 @@ able to efficiently skip non-competitive hits. <>:: A query that computes scores based on the dynamically computed distances -between the origin and documents' long numeric, geo or distance fields. +between the origin and documents' date, date_nanos and geo_point fields. It is able to efficiently skip non-competitive hits. <>:: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml index 0fc4c0d682041..528b03a1fbed4 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/250_distance_feature.yml @@ -11,10 +11,10 @@ setup: number_of_replicas: 0 mappings: properties: - my_long: - type: long my_date: type: date + my_date_nanos: + type: date_nanos my_geo: type: geo_point @@ -23,30 +23,11 @@ setup: refresh: true body: - '{ "index" : { "_index" : "index1", "_id" : "1" } }' - - '{ "my_long" : 22, "my_date": "2018-02-01T10:00:30Z", "my_geo": [-71.34, 41.13] }' + - '{ "my_date": "2018-02-01T10:00:00Z", "my_date_nanos": "2018-02-01T00:00:00.223456789Z", "my_geo": [-71.34, 41.13] }' - '{ "index" : { "_index" : "index1", "_id" : "2" } }' - - '{ "my_long" : 25, "my_date": "2018-02-01T11:00:30Z", "my_geo": [-71.34, 41.14] }' + - '{ "my_date": "2018-02-01T11:00:00Z", "my_date_nanos": "2018-02-01T00:00:00.123456789Z", "my_geo": [-71.34, 41.14] }' - '{ "index" : { "_index" : "index1", "_id" : "3" } }' - - '{ "my_long" : 19, "my_date": "2018-02-01T09:00:30Z", "my_geo": [-71.34, 41.12] }' - ---- -"test distance_feature query on long type": - - - do: - search: - rest_total_hits_as_int: true - index: index1 - body: - query: - distance_feature: - field: my_long - pivot: 5 - origin: 15 - - - length: { hits.hits: 3 } - - match: { hits.hits.0._id: "3" } - - match: { hits.hits.1._id: "1" } - - match: { hits.hits.2._id: "2" } + - '{ "my_date": "2018-02-01T09:00:00Z", "my_date_nanos": "2018-02-01T00:00:00.323456789Z", "my_geo": [-71.34, 41.12] }' --- "test distance_feature query on date type": @@ -67,6 +48,25 @@ setup: - match: { hits.hits.1._id: "1" } - match: { hits.hits.2._id: "2" } +--- +"test distance_feature query on date_nanos type": + +- do: + search: + rest_total_hits_as_int: true + index: index1 + body: + query: + distance_feature: + field: my_date_nanos + pivot: 100000000nanos + origin: 2018-02-01T00:00:00.323456789Z + +- length: { hits.hits: 3 } +- match: { hits.hits.0._id: "3" } +- match: { hits.hits.1._id: "1" } +- match: { hits.hits.2._id: "2" } + --- "test distance_feature query on geo_point type": diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 3ee49c3c6f72f..a45667b908d74 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java @@ -545,7 +545,18 @@ private static GeoPoint parseGeoHash(GeoPoint point, String geohash, EffectivePo } } - public static GeoPoint parseFromString(String val){ + /** + * Parse a {@link GeoPoint} from a string. The string must have one of the following forms: + * + *
    + *
  • Latitude, Longitude form: 
    "<latitude>,<longitude>"
    • + *
  • Geohash form:: 
    "<geohash>"
    • + *
+ * + * @param val a String to parse the value from + * @return new parsed {@link GeoPoint} + */ + public static GeoPoint parseFromString(String val) { GeoPoint point = new GeoPoint(); boolean ignoreZValue = false; if (val.contains(",")) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index 8d392ed6aedb3..84ffc738412e2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -308,6 +308,10 @@ public DateFormatter dateTimeFormatter() { return dateTimeFormatter; } + public Resolution resolution() { + return resolution; + } + void setDateTimeFormatter(DateFormatter formatter) { checkIfFrozen(); this.dateTimeFormatter = formatter; diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java index d3509f60c6bf7..a505609e6cb50 100644 --- a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -35,11 +35,10 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.GeoPointFieldMapper.GeoPointFieldType; import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType; -import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType; -import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType; import java.io.IOException; import java.util.Objects; @@ -48,6 +47,7 @@ /** * A query to boost scores based on their proximity to the given origin + * for date, date_nanos and geo_point field types */ public class DistanceFeatureQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "distance_feature"; @@ -57,65 +57,38 @@ public class DistanceFeatureQueryBuilder extends AbstractQueryBuilder PARSER = new ConstructingObjectParser<>( "distance_feature", false, - args -> new DistanceFeatureQueryBuilder((String) args[0], args[1], args[2]) + args -> new DistanceFeatureQueryBuilder((String) args[0], (Origin) args[1], (String) args[2]) ); static { PARSER.declareString(constructorArg(), FIELD_FIELD); - // origin: number for long fields; number or string for date fields; string, array, object for geo fields - PARSER.declareField(constructorArg(), DistanceFeatureQueryBuilder::originFromXContent, + // origin: number or string for date and date_nanos fields; string, array, object for geo fields + PARSER.declareField(constructorArg(), DistanceFeatureQueryBuilder.Origin::originFromXContent, ORIGIN_FIELD, ObjectParser.ValueType.OBJECT_ARRAY_STRING_OR_NUMBER); - //pivot: number for long fields; string for dates or geo fields - PARSER.declareField(constructorArg(), DistanceFeatureQueryBuilder::pivotFromXContent, - PIVOT_FIELD, ObjectParser.ValueType.LONG); + PARSER.declareString(constructorArg(), PIVOT_FIELD); declareStandardFields(PARSER); } - public DistanceFeatureQueryBuilder(String field, Object origin, Object pivot) { - this.field = field; - this.origin = origin; - this.pivot = pivot; + public DistanceFeatureQueryBuilder(String field, Origin origin, String pivot) { + this.field = Objects.requireNonNull(field); + this.origin = Objects.requireNonNull(origin); + this.pivot = Objects.requireNonNull(pivot); } public static DistanceFeatureQueryBuilder fromXContent(XContentParser parser) { - try { - DistanceFeatureQueryBuilder builder = PARSER.apply(parser, null); - return builder; - } catch (IllegalArgumentException e) { - throw new ParsingException(parser.getTokenLocation(), e.getMessage(), e); - } - } - - public static Object originFromXContent(XContentParser parser) throws IOException { - if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return parser.longValue(); - } else if(parser.currentToken() == XContentParser.Token.VALUE_STRING) { - return parser.text(); - } else if (parser.currentToken() == XContentParser.Token.START_OBJECT) { - return GeoUtils.parseGeoPoint(parser); - } else { // if currentToken equals to START_ARRAY) - return GeoUtils.parseGeoPoint(parser); - } - } - - public static Object pivotFromXContent(XContentParser parser) throws IOException { - if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - return parser.longValue(); - } else { // if currentToken equals to VALUE_STRING - return parser.text(); - } + return PARSER.apply(parser, null); } @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); builder.field(FIELD_FIELD.getPreferredName(), field); - builder.field(ORIGIN_FIELD.getPreferredName(), origin); + builder.field(ORIGIN_FIELD.getPreferredName(), origin.origin); builder.field(PIVOT_FIELD.getPreferredName(), pivot); printBoostAndQueryName(builder); builder.endObject(); @@ -124,15 +97,15 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep public DistanceFeatureQueryBuilder(StreamInput in) throws IOException { super(in); field = in.readString(); - origin = in.readGenericValue(); - pivot = in.readGenericValue(); + origin = new Origin(in); + pivot = in.readString(); } @Override protected void doWriteTo(StreamOutput out) throws IOException { out.writeString(field); - out.writeGenericValue(origin); - out.writeGenericValue(pivot); + origin.writeTo(out); + out.writeString(pivot); } @Override @@ -146,35 +119,35 @@ protected Query doToQuery(QueryShardContext context) throws IOException { if (fieldType == null) { throw new IllegalArgumentException("Can't run [" + NAME + "] query on unmapped fields!"); } + Object originObj = origin.origin(); if (fieldType instanceof DateFieldType) { - long originLong = (origin instanceof Long) ? (Long) origin : - ((DateFieldType) fieldType).parseToLong(origin, true, null, null, context); - TimeValue val = TimeValue.parseTimeValue((String)pivot, TimeValue.timeValueHours(24), + long originLong = (originObj instanceof Long) ? (Long) originObj : + ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); + TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); - long pivotLong = val.getMillis(); - return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotLong); - } else if ((fieldType instanceof NumberFieldType) && (fieldType.typeName() == NumberType.LONG.typeName())) { - long originLong = (Long) origin; - long pivotLong = (Long) pivot; - return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotLong); + if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { + return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotVal.getMillis()); + } else { // NANOSECONDS + return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotVal.getNanos()); + } } else if (fieldType instanceof GeoPointFieldType) { - GeoPoint originGeoPoint = (origin instanceof GeoPoint)? (GeoPoint) origin : GeoUtils.parseFromString((String) origin); - double pivotDouble = DistanceUnit.DEFAULT.parse((String) pivot, DistanceUnit.DEFAULT); + GeoPoint originGeoPoint = (originObj instanceof GeoPoint)? (GeoPoint) originObj : GeoUtils.parseFromString((String) originObj); + double pivotDouble = DistanceUnit.DEFAULT.parse(pivot, DistanceUnit.DEFAULT); return LatLonPoint.newDistanceFeatureQuery(field, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); } throw new IllegalArgumentException( - "Illegal data type! ["+ NAME + "] query can only be run on a long, date or geo-point data type!"); + "Illegal data type! ["+ NAME + "] query can only be run on a date, date_nanos or geo_point field type!"); } public String fieldName() { return field; } - public Object origin() { + public Origin origin() { return origin; } - public Object pivot() { + public String pivot() { return pivot; } @@ -185,6 +158,58 @@ protected int doHashCode() { @Override protected boolean doEquals(DistanceFeatureQueryBuilder other) { - return this.field.equals(other.field) && Objects.equals(this.origin, other.origin) && Objects.equals(this.pivot, other.pivot); + return this.field.equals(other.field) && Objects.equals(this.origin, other.origin) && this.pivot.equals(other.pivot); + } + + public static class Origin { + private final Object origin; + public Origin(Object origin) { + if ((origin instanceof Long) || (origin instanceof GeoPoint) || (origin instanceof String)) { + this.origin = origin; + } else { + throw new IllegalArgumentException("Illegal type for [origin]! Must be of type [long] or [string] for " + + "date and date_nanos origins," + "[geo_point] or [string] for geo_point origins!"); + } + } + + private static Origin originFromXContent(XContentParser parser) throws IOException { + if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { + return new Origin(parser.longValue()); + } else if(parser.currentToken() == XContentParser.Token.VALUE_STRING) { + return new Origin(parser.text()); + } else if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + return new Origin(GeoUtils.parseGeoPoint(parser)); + } else if (parser.currentToken() == XContentParser.Token.START_ARRAY) { + return new Origin(GeoUtils.parseGeoPoint(parser)); + } else { + throw new ParsingException(parser.getTokenLocation(), + "Illegal type while parsing [origin]! Must be [number] or [string] for date and date_nanos origins;" + + " or [string], [array], [object] for geo_point origins!"); + } + } + + private Origin(StreamInput in) throws IOException { + origin = in.readGenericValue(); + } + + private void writeTo(final StreamOutput out) throws IOException { + out.writeGenericValue(origin); + } + + Object origin() { + return origin; + } + + @Override + public final boolean equals(Object other) { + if ((other instanceof Origin) == false) return false; + Object otherOrigin = ((Origin) other).origin(); + return this.origin().equals(otherOrigin); + } + + @Override + public int hashCode() { + return Objects.hash(origin); + } } } diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java index b142b2a59d5ed..1bee97fab916a 100644 --- a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -32,31 +32,38 @@ import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; import org.joda.time.DateTime; +import org.elasticsearch.index.query.DistanceFeatureQueryBuilder.Origin; +import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType; +import static org.elasticsearch.common.time.DateUtils.toLong; + import java.io.IOException; +import java.time.Duration; +import java.time.Instant; import static org.hamcrest.Matchers.containsString; public class DistanceFeatureQueryBuilderTests extends AbstractQueryTestCase { @Override protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { - String field = randomFrom(LONG_FIELD_NAME, DATE_FIELD_NAME, GEO_POINT_FIELD_NAME); - Object origin; - Object pivot; + String field = randomFrom(DATE_FIELD_NAME, DATE_NANOS_FIELD_NAME, GEO_POINT_FIELD_NAME); + Origin origin; + String pivot; switch (field) { case GEO_POINT_FIELD_NAME: - origin = new GeoPoint(randomDouble(), randomDouble()); - origin = randomBoolean()? origin : ((GeoPoint) origin).geohash(); + GeoPoint point = new GeoPoint(randomDouble(), randomDouble()); + origin = randomBoolean() ? new Origin(point) : new Origin(point.geohash()); pivot = randomFrom(DistanceUnit.values()).toString(randomDouble()); break; case DATE_FIELD_NAME: - long randomDateMills = System.currentTimeMillis() - randomIntBetween(0, 1000000); - origin = randomBoolean() ? randomDateMills : new DateTime(randomDateMills).toString(); + long randomDateMills = System.currentTimeMillis() - randomLongBetween(0, 1_000_000); + origin = randomBoolean() ? new Origin(randomDateMills) : new Origin(new DateTime(randomDateMills).toString()); pivot = randomTimeValue(1, 1000, "d", "h", "ms", "s", "m"); break; - default: // LONG_FIELD_NAME - origin = randomLongBetween(1, Long.MAX_VALUE); - pivot = randomLongBetween(1, Long.MAX_VALUE); + default: // DATE_NANOS_FIELD_NAME + Instant randomDateNanos = Instant.now().minus(Duration.ofNanos(randomLongBetween(0, 100_000_000))); + origin = randomBoolean() ? new Origin(toLong(randomDateNanos)) : new Origin(randomDateNanos.toString()); + pivot = randomTimeValue(1, 100_000_000, "nanos"); break; } return new DistanceFeatureQueryBuilder(field, origin, pivot); @@ -65,54 +72,71 @@ protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { @Override protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { String fieldName = expectedFieldName(queryBuilder.fieldName()); - Object origin = queryBuilder.origin(); - Object pivot = queryBuilder.pivot(); + 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((String) pivot, DistanceUnit.DEFAULT); + double pivotDouble = DistanceUnit.DEFAULT.parse(pivot, DistanceUnit.DEFAULT); expectedQuery = LatLonPoint.newDistanceFeatureQuery(fieldName, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); - } else if (fieldName.equals(DATE_FIELD_NAME)) { + } else { // if (fieldName.equals(DATE_FIELD_NAME)) MapperService mapperService = context.getQueryShardContext().getMapperService(); MappedFieldType fieldType = mapperService.fullName(fieldName); long originLong = (origin instanceof Long) ? (Long) origin : - ((DateFieldMapper.DateFieldType) fieldType).parseToLong(origin, true, null, null, context.getQueryShardContext()); - TimeValue val = TimeValue.parseTimeValue((String)pivot, TimeValue.timeValueHours(24), + ((DateFieldType) fieldType).parseToLong(origin, true, null, null, context.getQueryShardContext()); + TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); - long pivotLong = val.getMillis(); - expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, boost, originLong, pivotLong); - } else { - long originLong = (Long) origin; - long pivotLong = (Long) pivot; + long pivotLong; + if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { + pivotLong = pivotVal.getMillis(); + } else { // NANOSECONDS + pivotLong = pivotVal.getNanos(); + } expectedQuery = LongPoint.newDistanceFeatureQuery(fieldName, boost, originLong, pivotLong); } assertEquals(expectedQuery, query); } - public void testFromJsonLongFieldType() throws IOException { + public void testFromJsonDateFieldType() throws IOException { + // origin as string + String origin = "2018-01-01T13:10:30Z"; + String pivot = "7d"; String json = "{\n" + " \"distance_feature\" : {\n" + - " \"field\": \""+ LONG_FIELD_NAME + "\",\n" + - " \"origin\": 40,\n" + - " \"pivot\" : 10,\n" + - " \"boost\" : 2.0\n" + + " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"origin\": \"" + origin + "\",\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 1.0\n" + " }\n" + "}"; DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); checkGeneratedJson(json, parsed); - assertEquals(json, 40L, parsed.origin()); - assertEquals(json, 10L, parsed.pivot()); - assertEquals(json, 2.0, parsed.boost(), 0.0001); + assertEquals(json, origin, parsed.origin().origin()); + assertEquals(json, pivot, parsed.pivot()); + assertEquals(json, 1.0, parsed.boost(), 0.0001); + + // origin as long + long originLong = 1514812230999L; + json = "{\n" + + " \"distance_feature\" : {\n" + + " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"origin\": " + originLong + ",\n" + + " \"pivot\" : \"" + pivot + "\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + parsed = (DistanceFeatureQueryBuilder) parseQuery(json); + assertEquals(json, originLong, parsed.origin().origin()); } - public void testFromJsonDateFieldType() throws IOException { + public void testFromJsonDateNanosFieldType() throws IOException { // origin as string - String origin = "2018-01-01T13:10:30Z"; - String pivot = "7d"; + String origin = "2018-01-01T13:10:30.323456789Z"; + String pivot = "100000000nanos"; String json = "{\n" + " \"distance_feature\" : {\n" + - " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"field\": \""+ DATE_NANOS_FIELD_NAME + "\",\n" + " \"origin\": \"" + origin + "\",\n" + " \"pivot\" : \"" + pivot + "\",\n" + " \"boost\" : 1.0\n" + @@ -120,22 +144,22 @@ public void testFromJsonDateFieldType() throws IOException { "}"; DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); checkGeneratedJson(json, parsed); - assertEquals(json, origin, parsed.origin()); + assertEquals(json, origin, parsed.origin().origin()); assertEquals(json, pivot, parsed.pivot()); assertEquals(json, 1.0, parsed.boost(), 0.0001); // origin as long - long originLong = 1514812230999L; + long originLong = 1517443200323456789L; json = "{\n" + " \"distance_feature\" : {\n" + - " \"field\": \""+ DATE_FIELD_NAME + "\",\n" + + " \"field\": \""+ DATE_NANOS_FIELD_NAME + "\",\n" + " \"origin\": " + originLong + ",\n" + " \"pivot\" : \"" + pivot + "\",\n" + " \"boost\" : 1.0\n" + " }\n" + "}"; parsed = (DistanceFeatureQueryBuilder) parseQuery(json); - assertEquals(json, originLong, parsed.origin()); + assertEquals(json, originLong, parsed.origin().origin()); } public void testFromJsonGeoFieldType() throws IOException { @@ -153,7 +177,7 @@ public void testFromJsonGeoFieldType() throws IOException { "}"; DistanceFeatureQueryBuilder parsed = (DistanceFeatureQueryBuilder) parseQuery(json); checkGeneratedJson(json, parsed); - assertEquals(json, origin.toString(), parsed.origin()); + assertEquals(json, origin.toString(), parsed.origin().origin()); assertEquals(json, pivot, parsed.pivot()); assertEquals(json, 2.0, parsed.boost(), 0.0001); @@ -167,7 +191,7 @@ public void testFromJsonGeoFieldType() throws IOException { " }\n" + "}"; parsed = (DistanceFeatureQueryBuilder) parseQuery(json); - assertEquals(json, origin, parsed.origin()); + assertEquals(json, origin, parsed.origin().origin()); // origin as object json = "{\n" + @@ -179,15 +203,15 @@ public void testFromJsonGeoFieldType() throws IOException { " }\n" + "}"; parsed = (DistanceFeatureQueryBuilder) parseQuery(json); - assertEquals(json, origin, parsed.origin()); + assertEquals(json, origin, parsed.origin().origin()); } public void testQueryFailsWithUnmappedField() { String query = "{\n" + " \"distance_feature\" : {\n" + " \"field\": \"random_unmapped_field\",\n" + - " \"origin\": 40,\n" + - " \"pivot\" : 10\n" + + " \"origin\": \"random_string\",\n" + + " \"pivot\" : \"random_string\"\n" + " }\n" + "}"; IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(query).toQuery(createShardContext())); @@ -199,10 +223,10 @@ public void testQueryFailsWithWrongFieldType() { " \"distance_feature\" : {\n" + " \"field\": \""+ INT_FIELD_NAME + "\",\n" + " \"origin\": 40,\n" + - " \"pivot\" : 10\n" + + " \"pivot\" : \"random_string\"\n" + " }\n" + "}"; IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(query).toQuery(createShardContext())); - assertThat(e.getMessage(), containsString("query can only be run on a long, date or geo-point data type!")); + assertThat(e.getMessage(), containsString("query can only be run on a date, date_nanos or geo_point field type!")); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index d1e0de67369dc..a9080c688f64f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -84,7 +84,8 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() { choice.equals(GEO_POINT_ALIAS_FIELD_NAME) || choice.equals(GEO_SHAPE_FIELD_NAME) || choice.equals(INT_RANGE_FIELD_NAME) || - choice.equals(DATE_RANGE_FIELD_NAME), + choice.equals(DATE_RANGE_FIELD_NAME) || + choice.equals(DATE_NANOS_FIELD_NAME), // TODO: needs testing for date_nanos type () -> getRandomFieldName()); Object[] values = new Object[randomInt(5)]; for (int i = 0; i < values.length; i++) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index cb28942658fb8..0f1a3fb3dcb7c 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -104,9 +104,9 @@ public abstract class AbstractBuilderTestCase extends ESTestCase { protected static final String INT_FIELD_NAME = "mapped_int"; protected static final String INT_ALIAS_FIELD_NAME = "mapped_int_field_alias"; protected static final String INT_RANGE_FIELD_NAME = "mapped_int_range"; - protected static final String LONG_FIELD_NAME = "mapped_long"; protected static final String DOUBLE_FIELD_NAME = "mapped_double"; protected static final String BOOLEAN_FIELD_NAME = "mapped_boolean"; + protected static final String DATE_NANOS_FIELD_NAME = "mapped_date_nanos"; protected static final String DATE_FIELD_NAME = "mapped_date"; protected static final String DATE_ALIAS_FIELD_NAME = "mapped_date_alias"; protected static final String DATE_RANGE_FIELD_NAME = "mapped_date_range"; @@ -115,11 +115,11 @@ public abstract class AbstractBuilderTestCase extends ESTestCase { protected static final String GEO_POINT_ALIAS_FIELD_NAME = "mapped_geo_point_alias"; protected static final String GEO_SHAPE_FIELD_NAME = "mapped_geo_shape"; protected static final String[] MAPPED_FIELD_NAMES = new String[]{STRING_FIELD_NAME, STRING_ALIAS_FIELD_NAME, - INT_FIELD_NAME, INT_RANGE_FIELD_NAME, LONG_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_NANOS_FIELD_NAME, DATE_FIELD_NAME, DATE_RANGE_FIELD_NAME, OBJECT_FIELD_NAME, GEO_POINT_FIELD_NAME, GEO_POINT_ALIAS_FIELD_NAME, GEO_SHAPE_FIELD_NAME}; protected static final String[] MAPPED_LEAF_FIELD_NAMES = new String[]{STRING_FIELD_NAME, STRING_ALIAS_FIELD_NAME, - INT_FIELD_NAME, INT_RANGE_FIELD_NAME, LONG_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_NANOS_FIELD_NAME, DATE_FIELD_NAME, DATE_RANGE_FIELD_NAME, GEO_POINT_FIELD_NAME, GEO_POINT_ALIAS_FIELD_NAME}; private static final Map ALIAS_TO_CONCRETE_FIELD_NAME = new HashMap<>(); @@ -389,9 +389,9 @@ public void onRemoval(ShardId shardId, Accountable accountable) { INT_FIELD_NAME, "type=integer", INT_ALIAS_FIELD_NAME, "type=alias,path=" + INT_FIELD_NAME, INT_RANGE_FIELD_NAME, "type=integer_range", - LONG_FIELD_NAME, "type=long", DOUBLE_FIELD_NAME, "type=double", BOOLEAN_FIELD_NAME, "type=boolean", + DATE_NANOS_FIELD_NAME, "type=date_nanos", DATE_FIELD_NAME, "type=date", DATE_ALIAS_FIELD_NAME, "type=alias,path=" + DATE_FIELD_NAME, DATE_RANGE_FIELD_NAME, "type=date_range", diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index a16f55e04d74a..bead5ae70dc18 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -58,6 +58,7 @@ import org.joda.time.DateTimeZone; import java.io.IOException; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.Deque; @@ -632,7 +633,7 @@ private QB copyQuery(QB query) throws IOException { /** * create a random value for either {@link AbstractQueryTestCase#BOOLEAN_FIELD_NAME}, {@link AbstractQueryTestCase#INT_FIELD_NAME}, * {@link AbstractQueryTestCase#DOUBLE_FIELD_NAME}, {@link AbstractQueryTestCase#STRING_FIELD_NAME} or - * {@link AbstractQueryTestCase#DATE_FIELD_NAME}, or a String value by default + * {@link AbstractQueryTestCase#DATE_FIELD_NAME} or {@link AbstractQueryTestCase#DATE_NANOS_FIELD_NAME} or a String value by default */ protected static Object getRandomValueForFieldName(String fieldName) { Object value; @@ -659,6 +660,9 @@ protected static Object getRandomValueForFieldName(String fieldName) { case DATE_FIELD_NAME: value = new DateTime(System.currentTimeMillis(), DateTimeZone.UTC).toString(); break; + case DATE_NANOS_FIELD_NAME: + value = Instant.now().toString(); + break; default: value = randomAlphaOfLengthBetween(1, 10); } @@ -711,6 +715,8 @@ protected static Fuzziness randomFuzziness(String fieldName) { return Fuzziness.build(1 + randomFloat() * 10); case DATE_FIELD_NAME: return Fuzziness.build(randomTimeValue()); + case DATE_NANOS_FIELD_NAME: + return Fuzziness.build(randomTimeValue()); default: if (randomBoolean()) { return Fuzziness.fromEdits(randomIntBetween(0, 2)); From a591bb4533aa423ca313e3db2ba43808a86e69e4 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Fri, 15 Mar 2019 15:59:28 -0400 Subject: [PATCH 3/7] Address Adrien and Christophs' comments --- .../query/DistanceFeatureQueryBuilder.java | 53 ++++++++++++------- .../DistanceFeatureQueryBuilderTests.java | 17 +++--- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java index a505609e6cb50..b86311d7ea255 100644 --- a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ObjectParser; @@ -117,12 +118,11 @@ public String getWriteableName() { protected Query doToQuery(QueryShardContext context) throws IOException { MappedFieldType fieldType = context.fieldMapper(field); if (fieldType == null) { - throw new IllegalArgumentException("Can't run [" + NAME + "] query on unmapped fields!"); + return Queries.newMatchNoDocsQuery("Can't run [" + NAME + "] query on unmapped fields!"); } Object originObj = origin.origin(); if (fieldType instanceof DateFieldType) { - long originLong = (originObj instanceof Long) ? (Long) originObj : - ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); + long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { @@ -131,23 +131,31 @@ protected Query doToQuery(QueryShardContext context) throws IOException { return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotVal.getNanos()); } } else if (fieldType instanceof GeoPointFieldType) { - GeoPoint originGeoPoint = (originObj instanceof GeoPoint)? (GeoPoint) originObj : GeoUtils.parseFromString((String) originObj); + GeoPoint originGeoPoint; + if (originObj instanceof GeoPoint) { + originGeoPoint = (GeoPoint) originObj; + } else if (originObj instanceof String) { + originGeoPoint = GeoUtils.parseFromString((String) originObj); + } else { + throw new IllegalArgumentException("Illegal type ["+ origin.getClass() + "] for [origin]! " + + "Must be of type [geo_point] or [string] for geo_point fields!"); + } double pivotDouble = DistanceUnit.DEFAULT.parse(pivot, DistanceUnit.DEFAULT); return LatLonPoint.newDistanceFeatureQuery(field, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); } - throw new IllegalArgumentException( - "Illegal data type! ["+ NAME + "] query can only be run on a date, date_nanos or geo_point field type!"); + throw new IllegalArgumentException("Illegal data type of [" + fieldType.typeName() + "]!"+ + "[" + NAME + "] query can only be run on a date, date_nanos or geo_point field type!"); } - public String fieldName() { + String fieldName() { return field; } - public Origin origin() { + Origin origin() { return origin; } - public String pivot() { + String pivot() { return pivot; } @@ -163,13 +171,17 @@ protected boolean doEquals(DistanceFeatureQueryBuilder other) { public static class Origin { private final Object origin; - public Origin(Object origin) { - if ((origin instanceof Long) || (origin instanceof GeoPoint) || (origin instanceof String)) { - this.origin = origin; - } else { - throw new IllegalArgumentException("Illegal type for [origin]! Must be of type [long] or [string] for " + - "date and date_nanos origins," + "[geo_point] or [string] for geo_point origins!"); - } + + public Origin(Long origin) { + this.origin = origin; + } + + public Origin(String origin) { + this.origin = origin; + } + + public Origin(GeoPoint origin) { + this.origin = origin; } private static Origin originFromXContent(XContentParser parser) throws IOException { @@ -183,8 +195,8 @@ private static Origin originFromXContent(XContentParser parser) throws IOExcepti return new Origin(GeoUtils.parseGeoPoint(parser)); } else { throw new ParsingException(parser.getTokenLocation(), - "Illegal type while parsing [origin]! Must be [number] or [string] for date and date_nanos origins;" + - " or [string], [array], [object] for geo_point origins!"); + "Illegal type while parsing [origin]! Must be [number] or [string] for date and date_nanos fields;" + + " or [string], [array], [object] for geo_point fields!"); } } @@ -211,5 +223,10 @@ public final boolean equals(Object other) { public int hashCode() { return Objects.hash(origin); } + + @Override + public String toString() { + return origin.toString(); + } } } diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java index 1bee97fab916a..ada56dd9ff72d 100644 --- a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.Query; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.mapper.DateFieldMapper; @@ -82,13 +83,13 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder, Que expectedQuery = LatLonPoint.newDistanceFeatureQuery(fieldName, boost, originGeoPoint.lat(), originGeoPoint.lon(), pivotDouble); } else { // if (fieldName.equals(DATE_FIELD_NAME)) MapperService mapperService = context.getQueryShardContext().getMapperService(); - MappedFieldType fieldType = mapperService.fullName(fieldName); + DateFieldType fieldType = (DateFieldType) mapperService.fullName(fieldName); long originLong = (origin instanceof Long) ? (Long) origin : - ((DateFieldType) fieldType).parseToLong(origin, true, null, null, context.getQueryShardContext()); + fieldType.parseToLong(origin, true, null, null, context.getQueryShardContext()); TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); long pivotLong; - if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { + if (fieldType.resolution() == DateFieldMapper.Resolution.MILLISECONDS) { pivotLong = pivotVal.getMillis(); } else { // NANOSECONDS pivotLong = pivotVal.getNanos(); @@ -206,16 +207,18 @@ public void testFromJsonGeoFieldType() throws IOException { assertEquals(json, origin, parsed.origin().origin()); } - public void testQueryFailsWithUnmappedField() { - String query = "{\n" + + public void testQueryFailsWithUnmappedField() throws IOException { + Query expectedQuery = Queries.newMatchNoDocsQuery( + "Can't run [" + DistanceFeatureQueryBuilder.NAME + "] query on unmapped fields!"); + String queryString = "{\n" + " \"distance_feature\" : {\n" + " \"field\": \"random_unmapped_field\",\n" + " \"origin\": \"random_string\",\n" + " \"pivot\" : \"random_string\"\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(query).toQuery(createShardContext())); - assertEquals("Can't run [" + DistanceFeatureQueryBuilder.NAME + "] query on unmapped fields!", e.getMessage()); + Query query = parseQuery(queryString).toQuery(createShardContext()); + assertEquals(expectedQuery, query); } public void testQueryFailsWithWrongFieldType() { From e3366da56acd92d95624afa876354fe3be9c07dc Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Fri, 15 Mar 2019 18:31:21 -0400 Subject: [PATCH 4/7] Revert back origin --- .../elasticsearch/index/query/DistanceFeatureQueryBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java index b86311d7ea255..402a6d89cd5bf 100644 --- a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -122,7 +122,8 @@ protected Query doToQuery(QueryShardContext context) throws IOException { } Object originObj = origin.origin(); if (fieldType instanceof DateFieldType) { - long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); + long originLong = (originObj instanceof Long) ? (Long) originObj : + ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { From a4bd771597867ca08cb0f9f920ee1b75c9e20d64 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Sun, 17 Mar 2019 19:32:47 -0400 Subject: [PATCH 5/7] Change to milliseconds for date_nanos --- .../index/query/DistanceFeatureQueryBuilder.java | 3 +-- .../query/DistanceFeatureQueryBuilderTests.java | 14 ++++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java index 402a6d89cd5bf..b86311d7ea255 100644 --- a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -122,8 +122,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { } Object originObj = origin.origin(); if (fieldType instanceof DateFieldType) { - long originLong = (originObj instanceof Long) ? (Long) originObj : - ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); + long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java index ada56dd9ff72d..17e3b2b4adfd0 100644 --- a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -28,7 +28,6 @@ import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.mapper.DateFieldMapper; -import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; @@ -63,7 +62,11 @@ protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { break; default: // DATE_NANOS_FIELD_NAME Instant randomDateNanos = Instant.now().minus(Duration.ofNanos(randomLongBetween(0, 100_000_000))); - origin = randomBoolean() ? new Origin(toLong(randomDateNanos)) : new Origin(randomDateNanos.toString()); + if (randomBoolean()) { + origin = new Origin(toLong(randomDateNanos) / 1_000_000); // get milliseconds since epoch + } else { + origin = new Origin(randomDateNanos.toString()); + } pivot = randomTimeValue(1, 100_000_000, "nanos"); break; } @@ -84,8 +87,7 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder, Que } else { // if (fieldName.equals(DATE_FIELD_NAME)) MapperService mapperService = context.getQueryShardContext().getMapperService(); DateFieldType fieldType = (DateFieldType) mapperService.fullName(fieldName); - long originLong = (origin instanceof Long) ? (Long) origin : - fieldType.parseToLong(origin, true, null, null, context.getQueryShardContext()); + long originLong = fieldType.parseToLong(origin, true, null, null, context.getQueryShardContext()); TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); long pivotLong; @@ -150,7 +152,7 @@ public void testFromJsonDateNanosFieldType() throws IOException { assertEquals(json, 1.0, parsed.boost(), 0.0001); // origin as long - long originLong = 1517443200323456789L; + long originLong = 1514812230999L; json = "{\n" + " \"distance_feature\" : {\n" + " \"field\": \""+ DATE_NANOS_FIELD_NAME + "\",\n" + @@ -207,7 +209,7 @@ public void testFromJsonGeoFieldType() throws IOException { assertEquals(json, origin, parsed.origin().origin()); } - public void testQueryFailsWithUnmappedField() throws IOException { + public void testQueryMatchNoDocsQueryWithUnmappedField() throws IOException { Query expectedQuery = Queries.newMatchNoDocsQuery( "Can't run [" + DistanceFeatureQueryBuilder.NAME + "] query on unmapped fields!"); String queryString = "{\n" + From 83f0c84ffeca105433c74eb5d21202efe542a14e Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 18 Mar 2019 16:32:11 -0400 Subject: [PATCH 6/7] Address Adrien's comments --- .../index/query/DistanceFeatureQueryBuilder.java | 9 ++++----- .../query/DistanceFeatureQueryBuilderTests.java | 13 ++++++------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java index b86311d7ea255..bdee4342923ba 100644 --- a/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilder.java @@ -123,8 +123,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { Object originObj = origin.origin(); if (fieldType instanceof DateFieldType) { long originLong = ((DateFieldType) fieldType).parseToLong(originObj, true, null, null, context); - TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), - DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); + TimeValue pivotVal = TimeValue.parseTimeValue(pivot, DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); if (((DateFieldType) fieldType).resolution() == DateFieldMapper.Resolution.MILLISECONDS) { return LongPoint.newDistanceFeatureQuery(field, boost, originLong, pivotVal.getMillis()); } else { // NANOSECONDS @@ -173,15 +172,15 @@ public static class Origin { private final Object origin; public Origin(Long origin) { - this.origin = origin; + this.origin = Objects.requireNonNull(origin); } public Origin(String origin) { - this.origin = origin; + this.origin = Objects.requireNonNull(origin); } public Origin(GeoPoint origin) { - this.origin = origin; + this.origin = Objects.requireNonNull(origin); } private static Origin originFromXContent(XContentParser parser) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java index 17e3b2b4adfd0..539159514fe58 100644 --- a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -34,11 +34,9 @@ import org.joda.time.DateTime; import org.elasticsearch.index.query.DistanceFeatureQueryBuilder.Origin; import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType; -import static org.elasticsearch.common.time.DateUtils.toLong; import java.io.IOException; -import java.time.Duration; import java.time.Instant; import static org.hamcrest.Matchers.containsString; @@ -56,15 +54,17 @@ protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { pivot = randomFrom(DistanceUnit.values()).toString(randomDouble()); break; case DATE_FIELD_NAME: - long randomDateMills = System.currentTimeMillis() - randomLongBetween(0, 1_000_000); + long randomDateMills = randomLongBetween(0, 2_000_000_000_000l); origin = randomBoolean() ? new Origin(randomDateMills) : new Origin(new DateTime(randomDateMills).toString()); pivot = randomTimeValue(1, 1000, "d", "h", "ms", "s", "m"); break; default: // DATE_NANOS_FIELD_NAME - Instant randomDateNanos = Instant.now().minus(Duration.ofNanos(randomLongBetween(0, 100_000_000))); + randomDateMills = randomLongBetween(0, 2_000_000_000_000l); if (randomBoolean()) { - origin = new Origin(toLong(randomDateNanos) / 1_000_000); // get milliseconds since epoch + origin = new Origin(randomDateMills); // nano_dates long accept milliseconds since epoch } else { + long randomNanos = randomLongBetween(0, 1_000_000l); + Instant randomDateNanos = Instant.ofEpochMilli(randomDateMills).plusNanos(randomNanos); origin = new Origin(randomDateNanos.toString()); } pivot = randomTimeValue(1, 100_000_000, "nanos"); @@ -88,8 +88,7 @@ protected void doAssertLuceneQuery(DistanceFeatureQueryBuilder queryBuilder, Que MapperService mapperService = context.getQueryShardContext().getMapperService(); DateFieldType fieldType = (DateFieldType) mapperService.fullName(fieldName); long originLong = fieldType.parseToLong(origin, true, null, null, context.getQueryShardContext()); - TimeValue pivotVal = TimeValue.parseTimeValue(pivot, TimeValue.timeValueHours(24), - DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); + TimeValue pivotVal = TimeValue.parseTimeValue(pivot, DistanceFeatureQueryBuilder.class.getSimpleName() + ".pivot"); long pivotLong; if (fieldType.resolution() == DateFieldMapper.Resolution.MILLISECONDS) { pivotLong = pivotVal.getMillis(); From be7a4f9bd385a61e0185e1d195334b4bcb7d5051 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 18 Mar 2019 22:10:14 -0400 Subject: [PATCH 7/7] Correct checkstyle --- .../index/query/DistanceFeatureQueryBuilderTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java index 539159514fe58..c2fcfdd7140e2 100644 --- a/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/DistanceFeatureQueryBuilderTests.java @@ -54,16 +54,16 @@ protected DistanceFeatureQueryBuilder doCreateTestQueryBuilder() { pivot = randomFrom(DistanceUnit.values()).toString(randomDouble()); break; case DATE_FIELD_NAME: - long randomDateMills = randomLongBetween(0, 2_000_000_000_000l); + long randomDateMills = randomLongBetween(0, 2_000_000_000_000L); origin = randomBoolean() ? new Origin(randomDateMills) : new Origin(new DateTime(randomDateMills).toString()); pivot = randomTimeValue(1, 1000, "d", "h", "ms", "s", "m"); break; default: // DATE_NANOS_FIELD_NAME - randomDateMills = randomLongBetween(0, 2_000_000_000_000l); + randomDateMills = randomLongBetween(0, 2_000_000_000_000L); if (randomBoolean()) { origin = new Origin(randomDateMills); // nano_dates long accept milliseconds since epoch } else { - long randomNanos = randomLongBetween(0, 1_000_000l); + long randomNanos = randomLongBetween(0, 1_000_000L); Instant randomDateNanos = Instant.ofEpochMilli(randomDateMills).plusNanos(randomNanos); origin = new Origin(randomDateNanos.toString()); }