Fix setting of readerGen in BytesRefOrdValComparator on nested docume…

…nts.

Sorting was broken on nested documents because the `missing(slot)` method
didn't correctly set the segment ordinal (readerGen), causing term ordinals to
be compared across segments.

Close #5986

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

@@ -101,20 +101,14 @@ public BytesRefOrdValComparator(IndexFieldData.WithOrdinals<?> indexFieldData, i
     @Override
     public int compare(int slot1, int slot2) {
         if (readerGen[slot1] == readerGen[slot2]) {
-            return Long.compare(ords[slot1], ords[slot2]);
+            final int res = Long.compare(ords[slot1], ords[slot2]);
+            assert Integer.signum(res) == Integer.signum(compareValues(values[slot1], values[slot2])) : values[slot1] + " " + values[slot2] + " " + ords[slot1] + " " + ords[slot2];
+            return res;
         }
 
         final BytesRef val1 = values[slot1];
         final BytesRef val2 = values[slot2];
-        if (val1 == null) {
-            if (val2 == null) {
-                return 0;
-            }
-            return -1;
-        } else if (val2 == null) {
-            return 1;
-        }
-        return val1.compareTo(val2);
+        return compareValues(val1, val2);
     }
 
     @Override
@@ -257,6 +251,7 @@ public void copy(int slot, int doc) {
         public void missing(int slot) {
             ords[slot] = missingOrd;
             values[slot] = missingValue;
+            readerGen[slot] = currentReaderGen;
         }
     }
 
@@ -351,7 +346,6 @@ public void setBottom(final int bottom) {
             }
             assert consistentInsertedOrd(termsIndex, bottomOrd, bottomValue);
         }
-        readerGen[bottomSlot] = currentReaderGen;
     }
 
     @Override

src/test/java/org/elasticsearch/index/fielddata/NoOrdinalsStringFieldDataTests.java

@@ -23,18 +23,15 @@
 import org.apache.lucene.index.IndexReader;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.fielddata.fieldcomparator.BytesRefFieldComparatorSource;
-import org.elasticsearch.search.MultiValueMode;
 import org.elasticsearch.index.mapper.FieldMapper.Names;
+import org.elasticsearch.search.MultiValueMode;
 import org.junit.Test;
 
 /** Returns an implementation based on paged bytes which doesn't implement WithOrdinals in order to visit different paths in the code,
  *  eg. BytesRefFieldComparatorSource makes decisions based on whether the field data implements WithOrdinals. */
 public class NoOrdinalsStringFieldDataTests extends PagedBytesStringFieldDataTests {
 
-    @SuppressWarnings("unchecked")
-    @Override
-    public IndexFieldData<AtomicFieldData<ScriptDocValues>> getForField(String fieldName) {
-        final IndexFieldData<?> in = super.getForField(fieldName);
+    public static IndexFieldData<AtomicFieldData<ScriptDocValues>> hideOrdinals(final IndexFieldData<?> in) {
         return new IndexFieldData<AtomicFieldData<ScriptDocValues>>() {
 
             @Override
@@ -85,6 +82,12 @@ public void clear(IndexReader reader) {
         };
     }
 
+    @SuppressWarnings("unchecked")
+    @Override
+    public IndexFieldData<AtomicFieldData<ScriptDocValues>> getForField(String fieldName) {
+        return hideOrdinals(super.getForField(fieldName));
+    }
+
     @Test
     @Override
     public void testTermsEnum() throws Exception {

src/test/java/org/elasticsearch/index/search/nested/NestedSortingTests.java

@@ -30,17 +30,19 @@
 import org.apache.lucene.search.join.ScoreMode;
 import org.apache.lucene.search.join.ToParentBlockJoinQuery;
 import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.TestUtil;
 import org.elasticsearch.common.lucene.search.AndFilter;
 import org.elasticsearch.common.lucene.search.NotFilter;
 import org.elasticsearch.common.lucene.search.XFilteredQuery;
 import org.elasticsearch.common.settings.ImmutableSettings;
-import org.elasticsearch.index.fielddata.AbstractFieldDataTests;
-import org.elasticsearch.index.fielddata.FieldDataType;
+import org.elasticsearch.index.fielddata.*;
+import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource;
 import org.elasticsearch.index.fielddata.fieldcomparator.BytesRefFieldComparatorSource;
-import org.elasticsearch.search.MultiValueMode;
 import org.elasticsearch.index.fielddata.plain.PagedBytesIndexFieldData;
+import org.elasticsearch.search.MultiValueMode;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -56,6 +58,61 @@ protected FieldDataType getFieldDataType() {
         return new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes"));
     }
 
+    @Test
+    public void testDuel() throws Exception {
+        final int numDocs = scaledRandomIntBetween(100, 1000);
+        for (int i = 0; i < numDocs; ++i) {
+            final int numChildren = randomInt(2);
+            List<Document> docs = new ArrayList<>(numChildren + 1);
+            for (int j = 0; j < numChildren; ++j) {
+                Document doc = new Document();
+                doc.add(new StringField("f", TestUtil.randomSimpleString(getRandom(), 2), Field.Store.NO));
+                doc.add(new StringField("__type", "child", Field.Store.NO));
+                docs.add(doc);
+            }
+            if (randomBoolean()) {
+                docs.add(new Document());
+            }
+            Document parent = new Document();
+            parent.add(new StringField("__type", "parent", Field.Store.NO));
+            docs.add(parent);
+            writer.addDocuments(docs);
+            if (rarely()) { // we need to have a bit more segments than what RandomIndexWriter would do by default
+                DirectoryReader.open(writer, false).close();
+            }
+        }
+        writer.commit();
+
+        MultiValueMode sortMode = randomFrom(Arrays.asList(MultiValueMode.MIN, MultiValueMode.MAX));
+        IndexSearcher searcher = new IndexSearcher(DirectoryReader.open(writer, false));
+        PagedBytesIndexFieldData indexFieldData1 = getForField("f");
+        IndexFieldData<?> indexFieldData2 = NoOrdinalsStringFieldDataTests.hideOrdinals(indexFieldData1);
+        final String missingValue = randomBoolean() ? null : TestUtil.randomSimpleString(getRandom(), 2);
+        final int n = randomIntBetween(1, numDocs + 2);
+        final boolean reverse = randomBoolean();
+
+        final TopDocs topDocs1 = getTopDocs(searcher, indexFieldData1, missingValue, sortMode, n, reverse);
+        final TopDocs topDocs2 = getTopDocs(searcher, indexFieldData2, missingValue, sortMode, n, reverse);
+        for (int i = 0; i < topDocs1.scoreDocs.length; ++i) {
+            final FieldDoc fieldDoc1 = (FieldDoc) topDocs1.scoreDocs[i];
+            final FieldDoc fieldDoc2 = (FieldDoc) topDocs2.scoreDocs[i];
+            assertEquals(fieldDoc1.doc, fieldDoc2.doc);
+            assertArrayEquals(fieldDoc1.fields, fieldDoc2.fields);
+        }
+
+        searcher.getIndexReader().close();
+    }
+
+    private TopDocs getTopDocs(IndexSearcher searcher, IndexFieldData<?> indexFieldData, String missingValue, MultiValueMode sortMode, int n, boolean reverse) throws IOException {
+        Filter parentFilter = new TermFilter(new Term("__type", "parent"));
+        Filter childFilter = new TermFilter(new Term("__type", "child"));
+        XFieldComparatorSource innerSource = indexFieldData.comparatorSource(missingValue, sortMode);
+        NestedFieldComparatorSource nestedComparatorSource = new NestedFieldComparatorSource(sortMode, innerSource, parentFilter, childFilter);
+        Query query = new ConstantScoreQuery(parentFilter);
+        Sort sort = new Sort(new SortField("f", nestedComparatorSource, reverse));
+        return searcher.search(query, n, sort);
+    }
+
     @Test
     public void testNestedSorting() throws Exception {
         List<Document> docs = new ArrayList<>();