Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix setting of readerGen in BytesRefOrdValComparator on nested documents. #5986

Closed
wants to merge 1 commit into from
Closed

Fix setting of readerGen in BytesRefOrdValComparator on nested documents. #5986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 5 additions & 11 deletions ...main/java/org/elasticsearch/index/fielddata/fieldcomparator/BytesRefOrdValComparator.java
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -351,7 +346,6 @@ public void setBottom(final int bottom) {
}
assert consistentInsertedOrd(termsIndex, bottomOrd, bottomValue);
}
readerGen[bottomSlot] = currentReaderGen;
}

@Override
Expand Down
13 changes: 8 additions & 5 deletions src/test/java/org/elasticsearch/index/fielddata/NoOrdinalsStringFieldDataTests.java
View file
Open in desktop
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
63 changes: 60 additions & 3 deletions src/test/java/org/elasticsearch/index/search/nested/NestedSortingTests.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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<>();
Expand Down