diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/DocValuesCollector.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/DocValuesCollector.java index 7e9b9c521233c..680c42a84f248 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/DocValuesCollector.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/DocValuesCollector.java @@ -92,22 +92,6 @@ public DocValuesCollector( boolean keepScores ) this.keepScores = keepScores; } - /** - * @return the documents matched by the query, one {@link MatchingDocs} per visited segment that contains a hit. - */ - public List getMatchingDocs() - { - if ( docs != null && segmentHits > 0 ) - { - matchingDocs.add( new MatchingDocs( this.context, docs.getDocIdSet(), segmentHits, scores ) ); - docs = null; - scores = null; - context = null; - } - - return Collections.unmodifiableList( matchingDocs ); - } - /** * @param field the field that contains the values * @return an iterator over all NumericDocValues from the given field @@ -129,7 +113,12 @@ public PrimitiveLongIterator getSortedValuesIterator( String field, Sort sort ) { return getValuesIterator( field ); } - TopDocs topDocs = getTopDocs( sort, getTotalHits() ); + int size = getTotalHits(); + if ( size == 0 ) + { + return PrimitiveLongCollections.emptyIterator(); + } + TopDocs topDocs = getTopDocs( sort, size ); LeafReaderContext[] contexts = getLeafReaderContexts( getMatchingDocs() ); return new TopDocsValuesIterator( topDocs, contexts, field ); } @@ -231,17 +220,35 @@ protected void doSetNextReader( LeafReaderContext context ) throws IOException { if ( docs != null && segmentHits > 0 ) { - matchingDocs.add( new MatchingDocs( this.context, docs.getDocIdSet(), segmentHits, scores ) ); + createMatchingDocs(); } - docs = createDocs( context.reader().maxDoc() ); + int maxDoc = context.reader().maxDoc(); + docs = createDocs( maxDoc ); if ( keepScores ) { - scores = new float[64]; // some initial size + int initialSize = Math.min( 32, maxDoc ); + scores = new float[initialSize]; } segmentHits = 0; this.context = context; } + /** + * @return the documents matched by the query, one {@link MatchingDocs} per visited segment that contains a hit. + */ + public List getMatchingDocs() + { + if ( docs != null && segmentHits > 0 ) + { + createMatchingDocs(); + docs = null; + scores = null; + context = null; + } + + return Collections.unmodifiableList( matchingDocs ); + } + /** * @return a new {@link Docs} to record hits. */ @@ -250,6 +257,26 @@ private Docs createDocs( final int maxDoc ) return new Docs( maxDoc ); } + private void createMatchingDocs() + { + if ( scores == null || scores.length == segmentHits ) + { + matchingDocs.add( new MatchingDocs( this.context, docs.getDocIdSet(), segmentHits, scores ) ); + } + else + { + // NOTE: we could skip the copy step here since the MatchingDocs are supposed to be + // consumed through any of the provided Iterators (actually, the replay method), + // which all don't care if scores has null values at the end. + // This is for just sanity's sake, we could also make MatchingDocs private + // and treat this as implementation detail. + float[] finalScores = new float[segmentHits]; + System.arraycopy( scores, 0, finalScores, 0, segmentHits ); + matchingDocs.add( new MatchingDocs( this.context, docs.getDocIdSet(), segmentHits, finalScores ) ); + } + } + + private TopDocs getTopDocs( Sort sort, int size ) throws IOException { TopDocs topDocs; @@ -304,10 +331,145 @@ private void replayTo( Collector collector ) throws IOException } } + /** + * Iterates over all per-segment {@link DocValuesCollector.MatchingDocs}. Supports two kinds of lookups. + * One, iterate over all long values of the given field (constructor argument). + * Two, lookup a value for the current doc in a sidecar {@code NumericDocValues} field. + * That is, this iterator has a main field, that drives the iteration and allow for lookups + * in other, secondary fields based on the current document of the main iteration. + * + * Lookups from this class are not thread-safe. Races can happen when the segment barrier + * is crossed; one thread might think it is reading from one segment while another thread has + * already advanced this Iterator to the next segment, having raced the first thread. + */ + public class LongValuesIterator extends PrimitiveLongCollections.PrimitiveLongBaseIterator implements DocValuesAccess + { + private final Iterator matchingDocs; + private final String field; + private final int size; + private DocIdSetIterator currentDisi; + private NumericDocValues currentDocValues; + private DocValuesCollector.MatchingDocs currentDocs; + private final Map docValuesCache; + + private int index = 0; + + /** + * @param allMatchingDocs all {@link DocValuesCollector.MatchingDocs} across all segments + * @param totalHits the total number of hits across all segments + * @param field the main field, whose values drive the iteration + */ + public LongValuesIterator( Iterable allMatchingDocs, int totalHits, String field ) + { + this.size = totalHits; + this.field = field; + matchingDocs = allMatchingDocs.iterator(); + docValuesCache = new HashMap<>(); + } + + /** + * @return the number of docs left in this iterator. + */ + public int remaining() + { + return size - index; + } + + @Override + public long current() + { + return next; + } + + @Override + public long getValue( String field ) + { + if ( ensureValidDisi() ) + { + if ( docValuesCache.containsKey( field ) ) + { + return docValuesCache.get( field ).get( currentDisi.docID() ); + } + + NumericDocValues docValues = currentDocs.readDocValues( field ); + docValuesCache.put( field, docValues ); + + return docValues.get( currentDisi.docID() ); + } + else + { + // same as DocValues.emptyNumeric()#get + // which means, getValue carries over the semantics of NDV + // -1 would also be a possibility here. + return 0; + } + } + + @Override + protected boolean fetchNext() + { + try + { + if ( ensureValidDisi() ) + { + int nextDoc = currentDisi.nextDoc(); + if ( nextDoc != DocIdSetIterator.NO_MORE_DOCS ) + { + index++; + return next( currentDocValues.get( nextDoc ) ); + } + else + { + currentDisi = null; + return fetchNext(); + } + } + } + catch ( IOException e ) + { + throw new RuntimeException( e ); + } + + return false; + } + + /** + * @return true if it was able to make sure, that currentDisi is valid + */ + private boolean ensureValidDisi() + { + try + { + while ( currentDisi == null ) + { + if ( matchingDocs.hasNext() ) + { + currentDocs = matchingDocs.next(); + currentDisi = currentDocs.docIdSet.iterator(); + if ( currentDisi != null ) + { + docValuesCache.clear(); + currentDocValues = currentDocs.readDocValues( field ); + } + } + else + { + return false; + } + } + return true; + } + catch ( IOException e ) + { + throw new RuntimeException( e ); + } + } + } + /** * Holds the documents that were matched per segment. */ - public final static class MatchingDocs + public static final class MatchingDocs { /** The {@code LeafReaderContext} for this segment. */ @@ -538,7 +700,7 @@ private static abstract class ScoreDocsIterator extends PrefetchingIterator( docs.scoreDocs ); + this.iterator = new ArrayIterator<>( docs.scoreDocs ); int segments = contexts.length; docStarts = new int[segments + 1]; for ( int i = 0; i < segments; i++ ) @@ -660,8 +822,12 @@ protected boolean fetchNext() private void loadNextValue( LeafReaderContext context, int docID ) { - NumericDocValues docValues = null; - if ( !docValuesCache.containsKey( context ) ) + NumericDocValues docValues; + if ( docValuesCache.containsKey( context ) ) + { + docValues = docValuesCache.get( context ); + } + else { try { diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LongValuesIterator.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LongValuesIterator.java deleted file mode 100644 index 94797eb1e2eb1..0000000000000 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LongValuesIterator.java +++ /dev/null @@ -1,165 +0,0 @@ -/* - * Copyright (c) 2002-2015 "Neo Technology," - * Network Engine for Objects in Lund AB [http://neotechnology.com] - * - * This file is part of Neo4j. - * - * Neo4j is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ -package org.neo4j.kernel.api.impl.index; - -import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.search.DocIdSetIterator; - -import java.io.IOException; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; - -import org.neo4j.collection.primitive.PrimitiveLongCollections.PrimitiveLongBaseIterator; - -/** - * Iterates over all per-segment {@link DocValuesCollector.MatchingDocs}. Supports two kinds of lookups. - * One, iterate over all long values of the given field (constructor argument). - * Two, lookup a value for the current doc in a sidecar {@code NumericDocValues} field. - * That is, this iterator has a main field, that drives the iteration and allow for lookups - * in other, secondary fields based on the current document of the main iteration. - * - * Lookups from this class are not thread-safe. Races can happen when the segment barrier - * is crossed; one thread might think it is reading from one segment while another thread has - * already advanced this Iterator to the next segment, having raced the first thread. - */ -public class LongValuesIterator extends PrimitiveLongBaseIterator implements DocValuesAccess -{ - private final Iterator matchingDocs; - private final String field; - private final int size; - private DocIdSetIterator currentDisi; - private NumericDocValues currentDocValues; - private DocValuesCollector.MatchingDocs currentDocs; - private final Map docValuesCache; - - private int index = 0; - - /** - * @param allMatchingDocs all {@link DocValuesCollector.MatchingDocs} across all segments - * @param totalHits the total number of hits across all segments - * @param field the main field, whose values drive the iteration - */ - public LongValuesIterator( Iterable allMatchingDocs, int totalHits, String field ) - { - this.size = totalHits; - this.field = field; - matchingDocs = allMatchingDocs.iterator(); - docValuesCache = new HashMap<>(); - } - - /** - * @return the number of docs left in this iterator. - */ - public int remaining() - { - return size - index; - } - - @Override - public long current() - { - return next; - } - - @Override - public long getValue( String field ) - { - if ( ensureValidDisi() ) - { - if ( docValuesCache.containsKey( field ) ) - { - return docValuesCache.get( field ).get( currentDisi.docID() ); - } - - NumericDocValues docValues = currentDocs.readDocValues( field ); - docValuesCache.put( field, docValues ); - - return docValues.get( currentDisi.docID() ); - } - else - { - // same as DocValues.emptyNumeric()#get - // which means, getValue carries over the semantics of NDV - // -1 would also be a possibility here. - return 0; - } - } - - @Override - protected boolean fetchNext() - { - try - { - if ( ensureValidDisi() ) - { - int nextDoc = currentDisi.nextDoc(); - if ( nextDoc != DocIdSetIterator.NO_MORE_DOCS ) - { - index++; - return next( currentDocValues.get( nextDoc ) ); - } - else - { - currentDisi = null; - return fetchNext(); - } - } - } - catch ( IOException e ) - { - throw new RuntimeException( e ); - } - - return false; - } - - /** - * @return true if it was able to make sure, that currentDisi is valid - */ - private boolean ensureValidDisi() - { - try - { - while ( currentDisi == null ) - { - if ( matchingDocs.hasNext() ) - { - currentDocs = matchingDocs.next(); - currentDisi = currentDocs.docIdSet.iterator(); - if ( currentDisi != null ) - { - docValuesCache.clear(); - currentDocValues = currentDocs.readDocValues( field ); - } - } - else - { - return false; - } - } - return true; - } - catch ( IOException e ) - { - throw new RuntimeException( e ); - } - } -} diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessorReader.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessorReader.java index c80cc31340670..ad77c154cc3dc 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessorReader.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessorReader.java @@ -222,8 +222,7 @@ protected PrimitiveLongIterator query( Query query ) { DocValuesCollector docValuesCollector = new DocValuesCollector(); searcher.search( query, docValuesCollector ); - return new LongValuesIterator( - docValuesCollector.getMatchingDocs(), docValuesCollector.getTotalHits(), NODE_ID_KEY ); + return docValuesCollector.getValuesIterator( NODE_ID_KEY ); } catch ( IOException e ) { diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/PageOfRangesIterator.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/PageOfRangesIterator.java index 6bd98ffe337c5..e62b30c9c95b7 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/PageOfRangesIterator.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/PageOfRangesIterator.java @@ -36,7 +36,7 @@ class PageOfRangesIterator extends PrefetchingIterator private final BitmapDocumentFormat format; private final int rangesPerPage; private final int[] labels; - private LongValuesIterator rangesIterator; + private DocValuesCollector.LongValuesIterator rangesIterator; PageOfRangesIterator( BitmapDocumentFormat format, IndexSearcher searcher, int rangesPerPage, Query query, int... labels ) @@ -60,7 +60,7 @@ protected PrimitiveLongIterator fetchNextOrNull() return null; // we are done searching with this iterator } - LongValuesIterator ranges = getRanges(); + DocValuesCollector.LongValuesIterator ranges = getRanges(); int pageSize = Math.min( ranges.remaining(), rangesPerPage ); long[] rangeMap = new long[pageSize * 2]; @@ -78,7 +78,7 @@ protected PrimitiveLongIterator fetchNextOrNull() return new LongPageIterator( new BitmapExtractor( format.bitmapFormat(), rangeMap ) ); } - private LongValuesIterator getRanges() { + private DocValuesCollector.LongValuesIterator getRanges() { if ( rangesIterator != null ) { return rangesIterator; diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/DocValuesCollectorTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/DocValuesCollectorTest.java new file mode 100644 index 0000000000000..a691e4a2a0e64 --- /dev/null +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/DocValuesCollectorTest.java @@ -0,0 +1,460 @@ +package org.neo4j.kernel.api.impl.index; + +import org.apache.lucene.document.Document; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.search.ConstantScoreScorer; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.search.Scorer; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; + +import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.graphdb.index.IndexHits; + +import static java.util.Collections.emptyList; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +public final class DocValuesCollectorTest +{ + @Test + public void shouldStartWithEmptyMatchingDocs() throws Exception + { + //given + DocValuesCollector collector = new DocValuesCollector(); + + // when + // then + assertEquals( emptyList(), collector.getMatchingDocs() ); + } + + @Test + public void shouldCollectAllHitsPerSegment() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector(); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 3 ); + collector.collect( 5 ); + collector.collect( 9 ); + + // then + assertEquals( 4, collector.getTotalHits() ); + List allMatchingDocs = collector.getMatchingDocs(); + assertEquals( 1, allMatchingDocs.size() ); + DocValuesCollector.MatchingDocs matchingDocs = allMatchingDocs.get( 0 ); + assertSame( readerStub.getContext(), matchingDocs.context ); + assertEquals( 4, matchingDocs.totalHits ); + DocIdSetIterator disi = matchingDocs.docIdSet.iterator(); + assertEquals( 1, disi.nextDoc() ); + assertEquals( 3, disi.nextDoc() ); + assertEquals( 5, disi.nextDoc() ); + assertEquals( 9, disi.nextDoc() ); + assertEquals( DocIdSetIterator.NO_MORE_DOCS, disi.nextDoc() ); + } + + @Test + public void shouldCollectOneMatchingDocsPerSegment() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector(); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 3 ); + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 5 ); + collector.collect( 9 ); + + // then + assertEquals( 4, collector.getTotalHits() ); + List allMatchingDocs = collector.getMatchingDocs(); + assertEquals( 2, allMatchingDocs.size() ); + + DocValuesCollector.MatchingDocs matchingDocs = allMatchingDocs.get( 0 ); + assertSame( readerStub.getContext(), matchingDocs.context ); + assertEquals( 2, matchingDocs.totalHits ); + DocIdSetIterator disi = matchingDocs.docIdSet.iterator(); + assertEquals( 1, disi.nextDoc() ); + assertEquals( 3, disi.nextDoc() ); + assertEquals( DocIdSetIterator.NO_MORE_DOCS, disi.nextDoc() ); + + matchingDocs = allMatchingDocs.get( 1 ); + assertSame( readerStub.getContext(), matchingDocs.context ); + assertEquals( 2, matchingDocs.totalHits ); + disi = matchingDocs.docIdSet.iterator(); + assertEquals( 5, disi.nextDoc() ); + assertEquals( 9, disi.nextDoc() ); + assertEquals( DocIdSetIterator.NO_MORE_DOCS, disi.nextDoc() ); + } + + @Test + public void shouldNotSaveScoresWhenNotRequired() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + + // then + DocValuesCollector.MatchingDocs matchingDocs = collector.getMatchingDocs().get( 0 ); + assertNull( matchingDocs.scores ); + } + + @Test + public void shouldSaveScoresWhenRequired() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 13.42f ) ); + collector.collect( 1 ); + + // then + DocValuesCollector.MatchingDocs matchingDocs = collector.getMatchingDocs().get( 0 ); + assertArrayEquals( new float[]{13.42f}, matchingDocs.scores, 0.0f ); + } + + @Test + public void shouldSaveScoresInADenseArray() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 1.0f ) ); + collector.collect( 1 ); + collector.setScorer( constantScorer( 41.0f ) ); + collector.collect( 41 ); + + // then + DocValuesCollector.MatchingDocs matchingDocs = collector.getMatchingDocs().get( 0 ); + assertArrayEquals( new float[]{1.0f, 41.0f}, matchingDocs.scores, 0.0f ); + } + + @Test + public void shouldDynamicallyResizeScoresArray() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 1.0f ) ); + // scores starts with array size of 32, adding 42 docs forces resize + for ( int i = 0; i < 42; i++ ) + { + collector.collect( i ); + } + + // then + DocValuesCollector.MatchingDocs matchingDocs = collector.getMatchingDocs().get( 0 ); + float[] scores = new float[42]; + Arrays.fill( scores, 1.0f ); + assertArrayEquals( scores, matchingDocs.scores, 0.0f ); + } + + @Test + public void shouldReturnIndexHitsInIndexOrderWhenNoSortIsGiven() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector(); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 2 ); + + // then + IndexHits indexHits = collector.getIndexHits( null ); + assertEquals( 2, indexHits.size() ); + assertEquals( "1", indexHits.next().get( "id" ) ); + assertEquals( "2", indexHits.next().get( "id" ) ); + assertFalse( indexHits.hasNext() ); + } + + @Test + public void shouldReturnIndexHitsOrderedByRelevance() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 1.0f ) ); + collector.collect( 1 ); + collector.setScorer( constantScorer( 2.0f ) ); + collector.collect( 2 ); + + + // then + IndexHits indexHits = collector.getIndexHits( Sort.RELEVANCE ); + assertEquals( 2, indexHits.size() ); + assertEquals( "2", indexHits.next().get( "id" ) ); + assertEquals( 2.0f, indexHits.currentScore(), 0.0f ); + assertEquals( "1", indexHits.next().get( "id" ) ); + assertEquals( 1.0f, indexHits.currentScore(), 0.0f ); + assertFalse( indexHits.hasNext() ); + } + + @Test + public void shouldReturnIndexHitsInGivenSortOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 43 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 3 ); + collector.collect( 37 ); + collector.collect( 42 ); + + // then + Sort byIdDescending = new Sort( new SortField( "id", SortField.Type.LONG, true ) ); + IndexHits indexHits = collector.getIndexHits( byIdDescending ); + assertEquals( 4, indexHits.size() ); + assertEquals( "42", indexHits.next().get( "id" ) ); + assertEquals( "37", indexHits.next().get( "id" ) ); + assertEquals( "3", indexHits.next().get( "id" ) ); + assertEquals( "1", indexHits.next().get( "id" ) ); + assertFalse( indexHits.hasNext() ); + } + + @Test + public void shouldSilentlyMergeAllSegments() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 2 ); + + // then + IndexHits indexHits = collector.getIndexHits( null ); + assertEquals( 2, indexHits.size() ); + assertEquals( "1", indexHits.next().get( "id" ) ); + assertEquals( "2", indexHits.next().get( "id" ) ); + assertFalse( indexHits.hasNext() ); + } + + @Test + public void shouldReturnEmptyIteratorWhenNoHits() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + + // then + IndexHits indexHits = collector.getIndexHits( null ); + assertEquals( 0, indexHits.size() ); + assertEquals( Float.NaN, indexHits.currentScore(), 0.0f ); + assertFalse( indexHits.hasNext() ); + } + + @Test + public void shouldReadDocValuesInIndexOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 2 ); + + // then + DocValuesCollector.LongValuesIterator valuesIterator = collector.getValuesIterator( "id" ); + assertEquals( 2, valuesIterator.remaining() ); + assertEquals( 1, valuesIterator.next() ); + assertEquals( 2, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldSilentlyMergeSegmentsWhenReadingDocValues() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 2 ); + + // then + DocValuesCollector.LongValuesIterator valuesIterator = collector.getValuesIterator( "id" ); + assertEquals( 2, valuesIterator.remaining() ); + assertEquals( 1, valuesIterator.next() ); + assertEquals( 2, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldReturnEmptyIteratorWhenNoDocValues() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + + // then + DocValuesCollector.LongValuesIterator valuesIterator = collector.getValuesIterator( "id" ); + assertEquals( 0, valuesIterator.remaining() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldReturnDocValuesInIndexOrderWhenNoSortIsGiven() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 2 ); + + // then + PrimitiveLongIterator valuesIterator = collector.getSortedValuesIterator( "id", null ); + assertEquals( 1, valuesIterator.next() ); + assertEquals( 2, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldReturnDocValuesInRelevanceOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 1.0f ) ); + collector.collect( 1 ); + collector.setScorer( constantScorer( 2.0f ) ); + collector.collect( 2 ); + + // then + PrimitiveLongIterator valuesIterator = collector.getSortedValuesIterator( "id", Sort.RELEVANCE ); + assertEquals( 2, valuesIterator.next() ); + assertEquals( 1, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldReturnDocValuesInGivenOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.collect( 1 ); + collector.collect( 2 ); + + // then + Sort byIdDescending = new Sort( new SortField( "id", SortField.Type.LONG, true ) ); + PrimitiveLongIterator valuesIterator = collector.getSortedValuesIterator( "id", byIdDescending ); + assertEquals( 2, valuesIterator.next() ); + assertEquals( 1, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldSilentlyMergeSegmentsWhenReturnDocValuesInOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( true ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 1.0f ) ); + collector.collect( 1 ); + collector.doSetNextReader( readerStub.getContext() ); + collector.setScorer( constantScorer( 2.0f ) ); + collector.collect( 2 ); + + // then + PrimitiveLongIterator valuesIterator = collector.getSortedValuesIterator( "id", Sort.RELEVANCE ); + assertEquals( 2, valuesIterator.next() ); + assertEquals( 1, valuesIterator.next() ); + assertFalse( valuesIterator.hasNext() ); + } + + @Test + public void shouldReturnEmptyIteratorWhenNoDocValuesInOrder() throws Exception + { + // given + DocValuesCollector collector = new DocValuesCollector( false ); + IndexReaderStub readerStub = indexReaderWithMaxDocs( 42 ); + + // when + collector.doSetNextReader( readerStub.getContext() ); + + // then + PrimitiveLongIterator valuesIterator = collector.getSortedValuesIterator( "id", Sort.RELEVANCE ); + assertFalse( valuesIterator.hasNext() ); + } + + private IndexReaderStub indexReaderWithMaxDocs( int maxDocs ) + { + NumericDocValues identityValues = new NumericDocValues() + { + @Override + public long get( int docID ) + { + return docID; + } + }; + IndexReaderStub stub = new IndexReaderStub( identityValues ); + stub.setElements( new String[maxDocs] ); + return stub; + } + + private Scorer constantScorer( float score ) + { + return new ConstantScoreScorer( null, score, (DocIdSetIterator) null ); + } +} diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/IndexReaderStub.java b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/IndexReaderStub.java index 70f97464a269d..8e50a59707826 100644 --- a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/IndexReaderStub.java +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/index/IndexReaderStub.java @@ -21,8 +21,11 @@ import org.apache.lucene.index.BinaryDocValues; import org.apache.lucene.index.DocValues; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.FieldInfos; import org.apache.lucene.index.Fields; +import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.index.SortedDocValues; @@ -32,6 +35,8 @@ import org.apache.lucene.util.Bits; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.Map; import org.neo4j.function.Function; @@ -52,6 +57,9 @@ public NumericDocValues apply( String s ) }; private IOException throwOnFields; + private static FieldInfo DummyFieldInfo = + new FieldInfo( "id", 0, false, true, false, IndexOptions.DOCS, + DocValuesType.NONE, -1, Collections.emptyMap() ); public IndexReaderStub( boolean allDeleted, final String... elements ) { @@ -76,7 +84,7 @@ public NumericDocValues apply( String s ) }; } - public IndexReaderStub( final Map ndvs ) + public IndexReaderStub( final Map ndvs ) { this.ndvs = new Function() { @@ -231,7 +239,7 @@ public int maxDoc() @Override public void document( int docID, StoredFieldVisitor visitor ) throws IOException { - + visitor.stringField( DummyFieldInfo, String.valueOf( docID ).getBytes( StandardCharsets.UTF_8 ) ); } @Override