Use numOrds rather than numDocs as upperbound for sorting

Closes #2773

src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java

@@ -449,11 +449,12 @@ public final class BytesRefOrdValComparator extends FieldComparator<BytesRef> {
     }
 
     final protected static int binarySearch(BytesValues.WithOrdinals a, BytesRef key) {
-        return binarySearch(a, key, 1, a.ordinals().getNumDocs() - 1);
+        return binarySearch(a, key, 1, a.ordinals().getNumOrds() - 1);
     }
 
     final protected static int binarySearch(BytesValues.WithOrdinals a, BytesRef key, int low, int high) {
-
+        assert a.getValueByOrd(high) == null | a.getValueByOrd(high) != null; // make sure we actually can get these values
+        assert a.getValueByOrd(low) == null | a.getValueByOrd(low) != null;
         while (low <= high) {
             int mid = (low + high) >>> 1;
             BytesRef midVal = a.getValueByOrd(mid);

src/test/java/org/elasticsearch/test/integration/search/sort/SimpleSortTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.test.integration.search.sort;
 
+import org.elasticsearch.ElasticSearchException;
 import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.ShardSearchFailure;
@@ -28,6 +29,8 @@ import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.text.Text;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.index.query.FilterBuilder;
+import org.elasticsearch.index.query.FilterBuilders;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
@@ -37,6 +40,7 @@ import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import java.io.IOException;
 import java.util.Arrays;
 
 import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
@@ -929,4 +933,115 @@ public class SimpleSortTests extends AbstractNodesTests {
         assertThat(((Text) searchResponse.getHits().getAt(2).sortValues()[0]).string(), equalTo("03"));
     }
     
+    @Test
+    public void testSortOnRareField() throws ElasticSearchException, IOException {
+        try {
+            client.admin().indices().prepareDelete("test").execute().actionGet();
+        } catch (Exception e) {
+            // ignore
+        }
+        client.admin().indices().prepareCreate("test")
+                .setSettings(ImmutableSettings.settingsBuilder().put("index.number_of_shards", 1).put("index.number_of_replicas", 0))
+                .addMapping("type1", XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("properties")
+                        .startObject("string_values").field("type", "string").field("index", "not_analyzed").endObject()
+                        .endObject().endObject().endObject())
+                .execute().actionGet();
+        client.admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
+        client.prepareIndex("test", "type1", Integer.toString(1)).setSource(jsonBuilder().startObject()
+                .array("string_values", "01", "05", "10", "08")
+                .endObject()).execute().actionGet();
+
+        
+       
+       
+        client.admin().indices().prepareRefresh().execute().actionGet();
+        SearchResponse searchResponse = client.prepareSearch()
+                .setQuery(matchAllQuery())
+                .setSize(3)
+                .addSort("string_values", SortOrder.DESC)
+                .execute().actionGet();
+
+        assertThat(searchResponse.getHits().hits().length, equalTo(1));
+
+
+        assertThat(searchResponse.getHits().getAt(0).id(), equalTo(Integer.toString(1)));
+        assertThat(((Text) searchResponse.getHits().getAt(0).sortValues()[0]).string(), equalTo("10"));
+        
+        client.prepareIndex("test", "type1", Integer.toString(2)).setSource(jsonBuilder().startObject()
+                .array("string_values", "11", "15", "20", "07")
+                .endObject()).execute().actionGet();
+        for (int i = 0; i < 15; i++) {
+            client.prepareIndex("test", "type1", Integer.toString(300+i)).setSource(jsonBuilder().startObject()
+                    .array("some_other_field", "foobar")
+                    .endObject()).execute().actionGet();
+        }
+        client.admin().indices().prepareRefresh().execute().actionGet();
+
+        searchResponse = client.prepareSearch()
+                .setQuery(matchAllQuery())
+                .setSize(2)
+                .addSort("string_values", SortOrder.DESC)
+                .execute().actionGet();
+
+        assertThat(searchResponse.getHits().hits().length, equalTo(2));
+
+        assertThat(searchResponse.getHits().getAt(0).id(), equalTo(Integer.toString(2)));
+        assertThat(((Text) searchResponse.getHits().getAt(0).sortValues()[0]).string(), equalTo("20"));
+
+        assertThat(searchResponse.getHits().getAt(1).id(), equalTo(Integer.toString(1)));
+        assertThat(((Text) searchResponse.getHits().getAt(1).sortValues()[0]).string(), equalTo("10"));
+
+        
+        client.prepareIndex("test", "type1", Integer.toString(3)).setSource(jsonBuilder().startObject()
+                .array("string_values", "02", "01", "03", "!4")
+                .endObject()).execute().actionGet();
+        for (int i = 0; i < 15; i++) {
+            client.prepareIndex("test", "type1", Integer.toString(300+i)).setSource(jsonBuilder().startObject()
+                    .array("some_other_field", "foobar")
+                    .endObject()).execute().actionGet();
+        }
+        client.admin().indices().prepareRefresh().execute().actionGet();
+
+        searchResponse = client.prepareSearch()
+                .setQuery(matchAllQuery())
+                .setSize(3)
+                .addSort("string_values", SortOrder.DESC)
+                .execute().actionGet();
+
+        assertThat(searchResponse.getHits().hits().length, equalTo(3));
+
+        assertThat(searchResponse.getHits().getAt(0).id(), equalTo(Integer.toString(2)));
+        assertThat(((Text) searchResponse.getHits().getAt(0).sortValues()[0]).string(), equalTo("20"));
+
+        assertThat(searchResponse.getHits().getAt(1).id(), equalTo(Integer.toString(1)));
+        assertThat(((Text) searchResponse.getHits().getAt(1).sortValues()[0]).string(), equalTo("10"));
+
+        assertThat(searchResponse.getHits().getAt(2).id(), equalTo(Integer.toString(3)));
+        assertThat(((Text) searchResponse.getHits().getAt(2).sortValues()[0]).string(), equalTo("03"));
+        
+        for (int i = 0; i < 15; i++) {
+            client.prepareIndex("test", "type1", Integer.toString(300+i)).setSource(jsonBuilder().startObject()
+                    .array("some_other_field", "foobar")
+                    .endObject()).execute().actionGet();
+            client.admin().indices().prepareRefresh().execute().actionGet();
+        }
+        
+        searchResponse = client.prepareSearch()
+                .setQuery(matchAllQuery())
+                .setSize(3)
+                .addSort("string_values", SortOrder.DESC)
+                .execute().actionGet();
+
+        assertThat(searchResponse.getHits().hits().length, equalTo(3));
+
+        assertThat(searchResponse.getHits().getAt(0).id(), equalTo(Integer.toString(2)));
+        assertThat(((Text) searchResponse.getHits().getAt(0).sortValues()[0]).string(), equalTo("20"));
+
+        assertThat(searchResponse.getHits().getAt(1).id(), equalTo(Integer.toString(1)));
+        assertThat(((Text) searchResponse.getHits().getAt(1).sortValues()[0]).string(), equalTo("10"));
+
+        assertThat(searchResponse.getHits().getAt(2).id(), equalTo(Integer.toString(3)));
+        assertThat(((Text) searchResponse.getHits().getAt(2).sortValues()[0]).string(), equalTo("03"));
+    }
+
 }