Skip to content

Commit

Permalink
[SORT] Use first non-empty result to detect if sort is required
Browse files Browse the repository at this point in the history 
Closes elastic#8226
  • Loading branch information
@s1monw
s1monw committed Oct 27, 2014
1 parent d66ab79 commit 8981e58
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 10 deletions.
38 changes: 28 additions & 10 deletions src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ public class SearchPhaseController extends AbstractComponent {
public static final Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>> QUERY_RESULT_ORDERING = new Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>>() {
@Override
public int compare(AtomicArray.Entry<? extends QuerySearchResultProvider> o1, AtomicArray.Entry<? extends QuerySearchResultProvider> o2) {
int i = o1.value.shardTarget().index().compareTo(o2.value.shardTarget().index());
final int i = o1.value.shardTarget().index().compareTo(o2.value.shardTarget().index());
if (i == 0) {
i = o1.value.shardTarget().shardId() - o2.value.shardTarget().shardId();
return o1.value.shardTarget().shardId() - o2.value.shardTarget().shardId();
}
return i;
}
Expand All @@ -85,6 +85,7 @@ public boolean optimizeSingleShard() {
return optimizeSingleShard;
}


public AggregatedDfs aggregateDfs(AtomicArray<DfsSearchResult> results) {
ObjectObjectOpenHashMap<Term, TermStatistics> termStatistics = HppcMaps.newNoNullKeysMap();
ObjectObjectOpenHashMap<String, CollectionStatistics> fieldStatistics = HppcMaps.newNoNullKeysMap();
Expand Down Expand Up @@ -198,11 +199,11 @@ public ScoreDoc[] sortDocs(boolean scrollSort, AtomicArray<? extends QuerySearch
@SuppressWarnings("unchecked")
AtomicArray.Entry<? extends QuerySearchResultProvider>[] sortedResults = results.toArray(new AtomicArray.Entry[results.size()]);
Arrays.sort(sortedResults, QUERY_RESULT_ORDERING);
QuerySearchResultProvider firstResult = sortedResults[0].value;

final QuerySearchResultProvider firstResult = sortedResults[0].value;
final TopDocs sortTopDocs = findSortTopDocs(results);
final Sort sort;
if (firstResult.queryResult().topDocs() instanceof TopFieldDocs) {
TopFieldDocs firstTopDocs = (TopFieldDocs) firstResult.queryResult().topDocs();
if (sortTopDocs instanceof TopFieldDocs) {
TopFieldDocs firstTopDocs = (TopFieldDocs) sortTopDocs;
sort = new Sort(firstTopDocs.fields);
} else {
sort = null;
Expand Down Expand Up @@ -265,6 +266,23 @@ public void fillDocIdsToLoad(AtomicArray<IntArrayList> docsIdsToLoad, ScoreDoc[]
}
}

/**
* Returns the first result that is either non-empty or an instance of TopFieldDocs. If a misbehaving node returns
* a empty TopDocs instance instead of empty TopFieldDocs due to an empty shards it's likely to show up as the first
* result in the list which needs to be skipped otherwise the entire merged search result is unsorted.
*/
private static TopDocs findSortTopDocs(List<? extends AtomicArray.Entry<? extends QuerySearchResultProvider>> results) {
assert results.size() > 0;
for (AtomicArray.Entry<? extends QuerySearchResultProvider> entry : results) {
TopDocs topDocs = entry.value.queryResult().topDocs();
if (topDocs.totalHits > 0 || topDocs instanceof TopFieldDocs) {
return topDocs;
}
}
// get the first one just in case if we didn't find anything
return results.get(0).value.queryResult().topDocs();
}

public InternalSearchResponse merge(ScoreDoc[] sortedDocs, AtomicArray<? extends QuerySearchResultProvider> queryResultsArr, AtomicArray<? extends FetchSearchResultProvider> fetchResultsArr) {

List<? extends AtomicArray.Entry<? extends QuerySearchResultProvider>> queryResults = queryResultsArr.asList();
Expand All @@ -274,13 +292,13 @@ public InternalSearchResponse merge(ScoreDoc[] sortedDocs, AtomicArray<? extends
return InternalSearchResponse.empty();
}

QuerySearchResult firstResult = queryResults.get(0).value.queryResult();

final QuerySearchResult firstResult = queryResults.get(0).value.queryResult();
boolean sorted = false;
int sortScoreIndex = -1;
if (firstResult.topDocs() instanceof TopFieldDocs) {
final TopDocs sortTopDocs = findSortTopDocs(queryResults);
if (sortTopDocs instanceof TopFieldDocs) {
sorted = true;
TopFieldDocs fieldDocs = (TopFieldDocs) firstResult.queryResult().topDocs();
TopFieldDocs fieldDocs = (TopFieldDocs) sortTopDocs;
for (int i = 0; i < fieldDocs.fields.length; i++) {
if (fieldDocs.fields[i].getType() == SortField.Type.SCORE) {
sortScoreIndex = i;
Expand Down
36 changes: 36 additions & 0 deletions src/test/java/org/elasticsearch/search/sort/SimpleSortTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.lucene.util.TestUtil;
import org.apache.lucene.util.UnicodeUtil;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
Expand Down Expand Up @@ -62,6 +63,41 @@
*/
public class SimpleSortTests extends ElasticsearchIntegrationTest {

public void testIssue8226() {
int numIndices = between(5, 10);
for (int i = 0; i < numIndices; i++) {
assertAcked(prepareCreate("test_" + i).addAlias(new Alias("test")));
if (i > 0) {
client().prepareIndex("test_" + i, "foo", "" + i).setSource("{\"entry\": " + i + "}").get();
}
}
ensureYellow();
refresh();
// sort DESC
SearchResponse searchResponse = client().prepareSearch()
.addSort(new FieldSortBuilder("entry").order(SortOrder.DESC).ignoreUnmapped(true))
.setSize(10).get();
assertSearchResponse(searchResponse);

for (int j = 1; j < searchResponse.getHits().hits().length; j++) {
Number current = (Number) searchResponse.getHits().hits()[j].getSource().get("entry");
Number previous = (Number) searchResponse.getHits().hits()[j-1].getSource().get("entry");
assertThat(searchResponse.toString(), current.intValue(), lessThan(previous.intValue()));
}

// sort ASC
searchResponse = client().prepareSearch()
.addSort(new FieldSortBuilder("entry").order(SortOrder.ASC).ignoreUnmapped(true))
.setSize(10).get();
assertSearchResponse(searchResponse);

for (int j = 1; j < searchResponse.getHits().hits().length; j++) {
Number current = (Number) searchResponse.getHits().hits()[j].getSource().get("entry");
Number previous = (Number) searchResponse.getHits().hits()[j-1].getSource().get("entry");
assertThat(searchResponse.toString(), current.intValue(), greaterThan(previous.intValue()));
}
}

public void testIssue6639() throws ExecutionException, InterruptedException {
assertAcked(prepareCreate("$index")
.addMapping("$type","{\"$type\": {\"_boost\": {\"name\": \"boost\", \"null_value\": 1.0}, \"properties\": {\"grantee\": {\"index\": \"not_analyzed\", \"term_vector\": \"with_positions_offsets\", \"type\": \"string\", \"analyzer\": \"snowball\", \"boost\": 1.0, \"store\": \"yes\"}}}}"));
Expand Down

0 comments on commit 8981e58

Please sign in to comment.