Skip to content

Commit

Permalink
Change the sort boolean option in percolate api to the sort dsl ava…
Browse files Browse the repository at this point in the history
…ilable in search api.

Closes #4625
  • Loading branch information
martijnvg committed Jan 9, 2014
1 parent 0973b28 commit 7e341ce
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 24 deletions.
6 changes: 4 additions & 2 deletions docs/reference/search/percolate.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,10 @@ occurred for the filter to included the latest percolate queries.
* `track_scores` - Whether the `_score` is included for each match. The is based on the query and represents how the query matched
to the percolate query's metadata and *not* how the document being percolated matched to the query. The `query` option
is required for this option. Defaults to `false`.
* `sort` - Whether the matches should be sorted by the `_score`. Similar to the `score` option, but also sorts the
matches. The `size` and `query` option are required for this option. Defaults to `false`.
* `sort` - Define a sort specification like in the search api. Currently only sorting `_score` reverse (default relevancy)
is supported. Other sort fields will throw an exception. The `size` and `query` option are required for this setting. Like
`track_score` the score is based on the query and represents how the query matched to the percolate query's metadata
and *not* how the document being percolated matched to the query.
* `facets` - Allows facet definitions to be included. The facets are based on the matching percolator queries. See facet
documentation how to define facets.
* `aggs` - Allows aggregation definitions to be included. The aggregations are based on the matching percolator queries,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.facet.FacetBuilder;
import org.elasticsearch.search.highlight.HighlightBuilder;
import org.elasticsearch.search.sort.SortBuilder;

import java.util.Map;

Expand Down Expand Up @@ -107,13 +108,21 @@ public PercolateRequestBuilder setSize(int size) {
}

/**
* Similar as {@link #setScore(boolean)}, but also sort by the score.
* Similar as {@link #setScore(boolean)}, but whether to sort by the score descending.
*/
public PercolateRequestBuilder setSort(boolean sort) {
public PercolateRequestBuilder setSortByScore(boolean sort) {
sourceBuilder().setSort(sort);
return this;
}

/**
* Adds
*/
public PercolateRequestBuilder addSort(SortBuilder sort) {
sourceBuilder().addSort(sort);
return this;
}

/**
* Whether to compute a score for each match and include it in the response. The score is based on
* {@link #setPercolateQuery(QueryBuilder)}}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.elasticsearch.search.builder.SearchSourceBuilderException;
import org.elasticsearch.search.facet.FacetBuilder;
import org.elasticsearch.search.highlight.HighlightBuilder;
import org.elasticsearch.search.sort.ScoreSortBuilder;
import org.elasticsearch.search.sort.SortBuilder;

import java.io.IOException;
import java.util.HashMap;
Expand All @@ -47,6 +49,7 @@ public class PercolateSourceBuilder implements ToXContent {
private FilterBuilder filterBuilder;
private Integer size;
private Boolean sort;
private List<SortBuilder> sorts;
private Boolean trackScores;
private HighlightBuilder highlightBuilder;
private List<FacetBuilder> facets;
Expand Down Expand Up @@ -105,10 +108,25 @@ public PercolateSourceBuilder setSize(int size) {
}

/**
* Similar as {@link #setTrackScores(boolean)}, but also sort by the score.
* Similar as {@link #setTrackScores(boolean)}, but whether to sort by the score descending.
*/
public PercolateSourceBuilder setSort(boolean sort) {
this.sort = sort;
if (sort) {
addSort(new ScoreSortBuilder());
} else {
this.sorts = null;
}
return this;
}

/**
* Adds a sort builder. Only sorting by score desc is supported.
*/
public PercolateSourceBuilder addSort(SortBuilder sort) {
if (sorts == null) {
sorts = Lists.newArrayList();
}
sorts.add(sort);
return this;
}

Expand Down Expand Up @@ -178,8 +196,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (size != null) {
builder.field("size", size);
}
if (sort != null) {
builder.field("sort", sort);
if (sorts != null) {
builder.startArray("sort");
for (SortBuilder sort : sorts) {
builder.startObject();
sort.toXContent(builder, params);
builder.endObject();
}
builder.endArray();
}
if (trackScores != null) {
builder.field("track_scores", trackScores);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public class PercolateContext extends SearchContext {

public boolean limit;
public int size;
public boolean sort;
public boolean doSort;
public byte percolatorTypeId;
private boolean trackScores;

Expand Down Expand Up @@ -112,6 +112,7 @@ public class PercolateContext extends SearchContext {
private SearchContextFacets facets;
private SearchContextAggregations aggregations;
private QuerySearchResult querySearchResult;
private Sort sort;

public PercolateContext(PercolateShardRequest request, SearchShardTarget searchShardTarget, IndexShard indexShard, IndexService indexService, CacheRecycler cacheRecycler, PageCacheRecycler pageCacheRecycler) {
this.request = request;
Expand Down Expand Up @@ -518,12 +519,13 @@ public Float minimumScore() {

@Override
public SearchContext sort(Sort sort) {
throw new UnsupportedOperationException();
this.sort = sort;
return this;
}

@Override
public Sort sort() {
throw new UnsupportedOperationException();
return sort;
}

@Override
Expand Down
27 changes: 23 additions & 4 deletions src/main/java/org/elasticsearch/percolator/PercolatorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import org.elasticsearch.search.highlight.HighlightPhase;
import org.elasticsearch.search.highlight.SearchContextHighlight;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.sort.SortParseElement;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -114,6 +115,7 @@ public class PercolatorService extends AbstractComponent {
private final FacetPhase facetPhase;
private final HighlightPhase highlightPhase;
private final AggregationPhase aggregationPhase;
private final SortParseElement sortParseElement;

@Inject
public PercolatorService(Settings settings, IndicesService indicesService, CacheRecycler cacheRecycler, PageCacheRecycler pageCacheRecycler,
Expand All @@ -127,6 +129,7 @@ public PercolatorService(Settings settings, IndicesService indicesService, Cache
this.highlightPhase = highlightPhase;
this.facetPhase = facetPhase;
this.aggregationPhase = aggregationPhase;
this.sortParseElement = new SortParseElement();

final long maxReuseBytes = settings.getAsBytesSize("indices.memory.memory_index.size_per_thread", new ByteSizeValue(1, ByteSizeUnit.MB)).bytes();
cache = new CloseableThreadLocal<MemoryIndex>() {
Expand Down Expand Up @@ -176,11 +179,11 @@ public PercolateShardResponse percolate(PercolateShardRequest request) {
throw new ElasticsearchIllegalArgumentException("Nothing to percolate");
}

if (context.percolateQuery() == null && (context.trackScores() || context.sort || context.facets() != null || context.aggregations() != null)) {
if (context.percolateQuery() == null && (context.trackScores() || context.doSort || context.facets() != null || context.aggregations() != null)) {
context.percolateQuery(new MatchAllDocsQuery());
}

if (context.sort && !context.limit) {
if (context.doSort && !context.limit) {
throw new ElasticsearchIllegalArgumentException("Can't sort if size isn't specified");
}

Expand Down Expand Up @@ -214,7 +217,7 @@ public PercolateShardResponse percolate(PercolateShardRequest request) {
if (request.onlyCount()) {
action = context.percolateQuery() != null ? queryCountPercolator : countPercolator;
} else {
if (context.sort) {
if (context.doSort) {
action = topMatchingPercolator;
} else if (context.percolateQuery() != null) {
action = context.trackScores() ? scoringPercolator : queryPercolator;
Expand Down Expand Up @@ -293,9 +296,15 @@ private ParsedDocument parseRequest(IndexService documentIndexService, Percolate
}
Filter filter = documentIndexService.queryParserService().parseInnerFilter(parser).filter();
context.percolateQuery(new XConstantScoreQuery(filter));
} else if ("sort".equals(currentFieldName)) {
parseSort(parser, context);
} else if (element != null) {
element.parse(parser, context);
}
} else if (token == XContentParser.Token.START_ARRAY) {
if ("sort".equals(currentFieldName)) {
parseSort(parser, context);
}
} else if (token == null) {
break;
} else if (token.isValue()) {
Expand All @@ -306,7 +315,7 @@ private ParsedDocument parseRequest(IndexService documentIndexService, Percolate
throw new ElasticsearchParseException("size is set to [" + context.size + "] and is expected to be higher or equal to 0");
}
} else if ("sort".equals(currentFieldName)) {
context.sort = parser.booleanValue();
parseSort(parser, context);
} else if ("track_scores".equals(currentFieldName) || "trackScores".equals(currentFieldName)) {
context.trackScores(parser.booleanValue());
}
Expand Down Expand Up @@ -359,6 +368,16 @@ private ParsedDocument parseRequest(IndexService documentIndexService, Percolate
return doc;
}

private void parseSort(XContentParser parser, PercolateContext context) throws Exception {
sortParseElement.parse(parser, context);
// null, means default sorting by relevancy
if (context.sort() == null) {
context.doSort = true;
} else {
throw new ElasticsearchParseException("Only _score desc is supported");
}
}

private ParsedDocument parseFetchedDoc(BytesReference fetchedDoc, IndexService documentIndexService, String type) {
ParsedDocument doc = null;
XContentParser parser = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
*/
public class SortParseElement implements SearchParseElement {

private static final SortField SORT_SCORE = new SortField(null, SortField.Type.SCORE);
public static final SortField SORT_SCORE = new SortField(null, SortField.Type.SCORE);
private static final SortField SORT_SCORE_REVERSE = new SortField(null, SortField.Type.SCORE, true);
private static final SortField SORT_DOC = new SortField(null, SortField.Type.DOC);
private static final SortField SORT_DOC_REVERSE = new SortField(null, SortField.Type.DOC, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void testFacetsAndAggregations() throws Exception {
if (randomBoolean()) {
percolateRequestBuilder.setScore(true);
} else {
percolateRequestBuilder.setSort(true).setSize(numQueries);
percolateRequestBuilder.setSortByScore(true).setSize(numQueries);
}

boolean countOnly = randomBoolean();
Expand Down
41 changes: 34 additions & 7 deletions src/test/java/org/elasticsearch/percolator/PercolatorTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
import org.elasticsearch.index.query.FilterBuilders;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.functionscore.factor.FactorBuilder;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.highlight.HighlightBuilder;
import org.elasticsearch.search.sort.SortBuilders;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.junit.Test;

Expand Down Expand Up @@ -1181,7 +1183,7 @@ public void testPercolateScoreAndSorting() throws Exception {
for (int i = 0; i < runs; i++) {
int size = randomIntBetween(1, 10);
PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setSize(size)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
Expand All @@ -1205,7 +1207,7 @@ public void testPercolateScoreAndSorting() throws Exception {
NavigableSet<Integer> levels = controlMap.get(value);
int size = randomIntBetween(1, levels.size());
PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setSize(size)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchQuery("field1", value), scriptFunction("doc['level'].value")))
Expand Down Expand Up @@ -1237,7 +1239,7 @@ public void testPercolateSortingWithNoSize() throws Exception {
refresh();

PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setSize(2)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
Expand All @@ -1249,7 +1251,7 @@ public void testPercolateSortingWithNoSize() throws Exception {
assertThat(response.getMatches()[1].getScore(), equalTo(1f));

response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
.execute().actionGet();
Expand All @@ -1262,13 +1264,38 @@ public void testPercolateSortingWithNoSize() throws Exception {
assertThat(response.getShardFailures()[1].reason(), containsString("Can't sort if size isn't specified"));
}

@Test
public void testPercolateSorting_unsupportedField() throws Exception {
client().admin().indices().prepareCreate("my-index").execute().actionGet();
ensureGreen();

client().prepareIndex("my-index", PercolatorService.TYPE_NAME, "1")
.setSource(jsonBuilder().startObject().field("query", matchAllQuery()).field("level", 1).endObject())
.execute().actionGet();
client().prepareIndex("my-index", PercolatorService.TYPE_NAME, "2")
.setSource(jsonBuilder().startObject().field("query", matchAllQuery()).field("level", 2).endObject())
.execute().actionGet();
refresh();

PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSize(2)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
.addSort(SortBuilders.fieldSort("level"))
.get();

assertThat(response.getShardFailures().length, equalTo(5));
assertThat(response.getShardFailures()[0].status(), equalTo(RestStatus.BAD_REQUEST));
assertThat(response.getShardFailures()[0].reason(), containsString("Only _score desc is supported"));
}

@Test
public void testPercolateOnEmptyIndex() throws Exception {
client().admin().indices().prepareCreate("my-index").execute().actionGet();
ensureGreen();

PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setSize(2)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
Expand All @@ -1288,7 +1315,7 @@ public void testPercolateNotEmptyIndexButNoRefresh() throws Exception {
.execute().actionGet();

PercolateResponse response = client().preparePercolate().setIndices("my-index").setDocumentType("my-type")
.setSort(true)
.setSortByScore(true)
.setSize(2)
.setPercolateDoc(docBuilder().setDoc("field", "value"))
.setPercolateQuery(QueryBuilders.functionScoreQuery(matchAllQuery(), scriptFunction("doc['level'].value")))
Expand Down Expand Up @@ -1449,7 +1476,7 @@ public int compare(PercolateResponse.Match a, PercolateResponse.Match b) {
.setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "The quick brown fox jumps over the lazy dog").endObject()))
.setHighlightBuilder(new HighlightBuilder().field("field1"))
.setPercolateQuery(functionScoreQuery(matchAllQuery()).add(new FactorBuilder().boostFactor(5.5f)))
.setSort(true)
.setSortByScore(true)
.execute().actionGet();
assertMatchCount(response, 5l);
assertThat(response.getMatches(), arrayWithSize(5));
Expand Down

0 comments on commit 7e341ce

Please sign in to comment.