Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mayya-sharipova committed Jul 23, 2020
1 parent 466d3db commit 6827412
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lucene/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Optimizations
instructs Lucene to skip non-competitive documents whenever possible. For numeric
sort fields the skipping functionality works when the same field is indexed both
with doc values and points. To indicate that the same data is stored in these points
and doc values SortField#setCanSkipNonCompetitiveDocs method should be used.
and doc values SortField#setCanUsePoints method should be used.
(Mayya Sharipova, Jim Ferenczi, Adrien Grand)

Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private FieldValueHitQueue(SortField[] fields, int size, boolean filterNonCompet
for (int i = 0; i < numComparators; ++i) {
SortField field = fields[i];
reverseMul[i] = field.reverse ? -1 : 1;
if (i == 0 && field.getSkipNonCompetitiveDocs() && filterNonCompetitiveDocs) {
if (i == 0 && field.getCanUsePoints() && filterNonCompetitiveDocs) {
// try to rewrite the 1st comparator to the comparator that can skip non-competitive documents
// skipping functionality is beneficial only for the 1st comparator
comparators[i] = FilteringFieldComparator.wrapToFilteringComparator(field.getComparator(size, i),
Expand Down
19 changes: 9 additions & 10 deletions lucene/core/src/java/org/apache/lucene/search/SortField.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ public static enum Type {
// Used for 'sortMissingFirst/Last'
protected Object missingValue = null;

// For numeric sort fields, if true, indicates that the same data has been
// indexed with doc values and points, which allows to use sort optimization
// and skip non-competitive documents.
private boolean skipNonCompetitiveDocs = false;
// For numeric sort fields, if true, indicates the DocValues and Point fields
// with the same name have the exactly same data indexed.
// This allows to use them to optimize sort and skip non-competitive documents.
private boolean setCanUsePoints = false;

/** Creates a sort by terms in the given field with the type of term
* values explicitly given.
Expand Down Expand Up @@ -406,12 +406,12 @@ public String toString() {
* that these fields have the same name.
* This allows to use sort optimization and skip non-competitive documents.
*/
public void setSkipNonCompetitiveDocs() {
this.skipNonCompetitiveDocs = true;
public void setCanUsePoints() {
this.setCanUsePoints = true;
}

public boolean getSkipNonCompetitiveDocs() {
return skipNonCompetitiveDocs;
public boolean getCanUsePoints() {
return setCanUsePoints;
}

/** Returns true if <code>o</code> is equal to this. If a
Expand All @@ -428,7 +428,6 @@ public boolean equals(Object o) {
&& other.reverse == this.reverse
&& Objects.equals(this.comparatorSource, other.comparatorSource)
&& Objects.equals(this.missingValue, other.missingValue)
&& other.skipNonCompetitiveDocs == this.skipNonCompetitiveDocs
);
}

Expand All @@ -437,7 +436,7 @@ public boolean equals(Object o) {
* implement hashCode (unless a singleton is always used). */
@Override
public int hashCode() {
return Objects.hash(field, type, reverse, comparatorSource, missingValue, skipNonCompetitiveDocs);
return Objects.hash(field, type, reverse, comparatorSource, missingValue);
}

private Comparator<BytesRef> bytesComparator = Comparator.naturalOrder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void testLongSortOptimization() throws IOException {
final IndexReader reader = DirectoryReader.open(writer);
IndexSearcher searcher = new IndexSearcher(reader);
final SortField sortField = new SortField("my_field", SortField.Type.LONG);
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final int numHits = 3;
final int totalHitsThreshold = 3;
Expand Down Expand Up @@ -98,7 +98,7 @@ public void testLongSortOptimization() throws IOException {
assertTrue(topDocs.totalHits.value < numDocs);
}

{ // test that by default (if we don't set sortField.setSkipNonCompetitiveDocs()), the optimization is not run
{ // test that by default (if we don't set sortField.setCanUsePoints()), the optimization is not run
final SortField sortField2 = new SortField("my_field", SortField.Type.LONG);
final Sort sort2 = new Sort(sortField2);
final TopFieldCollector collector = TopFieldCollector.create(sort2, numHits, null, totalHitsThreshold);
Expand Down Expand Up @@ -132,7 +132,7 @@ public void testLongSortOptimizationOnFieldNotIndexedWithPoints() throws IOExcep
final IndexReader reader = DirectoryReader.open(writer);
IndexSearcher searcher = new IndexSearcher(reader);
final SortField sortField = new SortField("my_field", SortField.Type.LONG);
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final int numHits = 3;
final int totalHitsThreshold = 3;
Expand Down Expand Up @@ -174,7 +174,7 @@ public void testSortOptimizationWithMissingValues() throws IOException {
{ // test that optimization is not run when missing value setting of SortField is competitive
final SortField sortField = new SortField("my_field", SortField.Type.LONG);
sortField.setMissingValue(0L); // set a competitive missing value
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final TopFieldCollector collector = TopFieldCollector.create(sort, numHits, null, totalHitsThreshold);
searcher.search(new MatchAllDocsQuery(), collector);
Expand All @@ -185,7 +185,7 @@ public void testSortOptimizationWithMissingValues() throws IOException {
{ // test that optimization is run when missing value setting of SortField is NOT competitive
final SortField sortField = new SortField("my_field", SortField.Type.LONG);
sortField.setMissingValue(100L); // set a NON competitive missing value
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final TopFieldCollector collector = TopFieldCollector.create(sort, numHits, null, totalHitsThreshold);
searcher.search(new MatchAllDocsQuery(), collector);
Expand Down Expand Up @@ -218,7 +218,7 @@ public void testSortOptimizationEqualValues() throws IOException {

{ // test that sorting on a single field with equal values uses the optimization
final SortField sortField = new SortField("my_field1", SortField.Type.INT);
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final TopFieldCollector collector = TopFieldCollector.create(sort, numHits, null, totalHitsThreshold);
searcher.search(new MatchAllDocsQuery(), collector);
Expand All @@ -234,7 +234,7 @@ public void testSortOptimizationEqualValues() throws IOException {
{ // test that sorting on a single field with equal values and after parameter uses the optimization
final int afterValue = 100;
final SortField sortField = new SortField("my_field1", SortField.Type.INT);
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
FieldDoc after = new FieldDoc(10, Float.NaN, new Integer[] {afterValue});
final TopFieldCollector collector = TopFieldCollector.create(sort, numHits, after, totalHitsThreshold);
Expand All @@ -250,7 +250,7 @@ public void testSortOptimizationEqualValues() throws IOException {

{ // test that sorting on main field with equal values + another field for tie breaks doesn't use optimization
final SortField sortField1 = new SortField("my_field1", SortField.Type.INT);
sortField1.setSkipNonCompetitiveDocs();
sortField1.setCanUsePoints();
final SortField sortField2 = new SortField("my_field2", SortField.Type.INT);
final Sort sort = new Sort(sortField1, sortField2);
final TopFieldCollector collector = TopFieldCollector.create(sort, numHits, null, totalHitsThreshold);
Expand Down Expand Up @@ -286,7 +286,7 @@ public void testFloatSortOptimization() throws IOException {
final IndexReader reader = DirectoryReader.open(writer);
IndexSearcher searcher = new IndexSearcher(reader);
final SortField sortField = new SortField("my_field", SortField.Type.FLOAT);
sortField.setSkipNonCompetitiveDocs();
sortField.setCanUsePoints();
final Sort sort = new Sort(sortField);
final int numHits = 3;
final int totalHitsThreshold = 3;
Expand Down

0 comments on commit 6827412

Please sign in to comment.