honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix setting of readerGen in BytesRefOrdValComparator on nested documents. 

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
This commit is contained in:
Adrien Grand 2014-04-29 19:52:35 +02:00
parent 2076194d8f
commit 2eeaa56d95
3 changed files with 73 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java
View File

@ -101,20 +101,14 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator<By
@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 final class BytesRefOrdValComparator extends NestedWrappableComparator<By
public void missing(int slot) {
ords[slot] = missingOrd;
values[slot] = missingValue;
readerGen[slot] = currentReaderGen;
}
}
@ -351,7 +346,6 @@ public final class BytesRefOrdValComparator extends NestedWrappableComparator<By
}
assert consistentInsertedOrd(termsIndex, bottomOrd, bottomValue);
}
readerGen[bottomSlot] = currentReaderGen;
}
@Override

13
src/test/java/org/elasticsearch/index/fielddata/NoOrdinalsStringFieldDataTests.java
View File

@ -23,18 +23,15 @@ import org.apache.lucene.index.AtomicReaderContext;
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 class NoOrdinalsStringFieldDataTests extends PagedBytesStringFieldDataTes
};
}
@SuppressWarnings("unchecked")
@Override
public IndexFieldData<AtomicFieldData<ScriptDocValues>> getForField(String fieldName) {
return hideOrdinals(super.getForField(fieldName));
}
@Test
@Override
public void testTermsEnum() throws Exception {

63
src/test/java/org/elasticsearch/index/search/nested/NestedSortingTests.java
View File

@ -30,17 +30,19 @@ import org.apache.lucene.search.join.FixedBitSetCachingWrapperFilter;
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 @@ public class NestedSortingTests extends AbstractFieldDataTests {
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<>();