Index: lucene/core/src/test/org/apache/lucene/search/TestSort.java =================================================================== --- lucene/core/src/test/org/apache/lucene/search/TestSort.java (revision 1381237) +++ lucene/core/src/test/org/apache/lucene/search/TestSort.java (working copy) @@ -1039,7 +1039,7 @@ TopDocs td = tdc.topDocs(); ScoreDoc[] sd = td.scoreDocs; - assertEquals(10, sd.length); + assertEquals("collector=" + tdc + " sort=" + sort[i], 10, sd.length); } } } Index: lucene/core/src/java/org/apache/lucene/search/BooleanScorer.java =================================================================== --- lucene/core/src/java/org/apache/lucene/search/BooleanScorer.java (revision 1381237) +++ lucene/core/src/java/org/apache/lucene/search/BooleanScorer.java (working copy) @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import org.apache.lucene.index.AtomicReaderContext; @@ -60,54 +61,119 @@ final class BooleanScorer extends Scorer { - private static final class BooleanScorerCollector extends Collector { - private BucketTable bucketTable; - private int mask; - private Scorer scorer; + private static abstract class BooleanScorerCollectorBase extends Collector { + protected final BucketTable table; + protected final Scorer scorer; + + protected BooleanScorerCollectorBase(Scorer scorer, BucketTable table) { + this.scorer = scorer; + this.table = table; + } + + @Override + public void setNextReader(AtomicReaderContext context) { + // not needed by this implementation + } - public BooleanScorerCollector(int mask, BucketTable bucketTable) { - this.mask = mask; - this.bucketTable = bucketTable; + @Override + public void setScorer(Scorer scorer) { + // nocommit why does this trip... + // assert scorer == this.scorer: "scorer=" + scorer + " vs this.scorer=" + this.scorer; } @Override + public boolean acceptsDocsOutOfOrder() { + return true; + } + } + + // First, not prohibited + private static final class FirstBooleanScorerCollector extends BooleanScorerCollectorBase { + + public FirstBooleanScorerCollector(Scorer scorer, BucketTable table) { + super(scorer, table); + } + + @Override public void collect(final int doc) throws IOException { - final BucketTable table = bucketTable; - final int i = doc & BucketTable.MASK; - final Bucket bucket = table.buckets[i]; - + //System.out.println(" collect doc=" + doc); + final int slot = doc & BucketTable.MASK; + final Bucket bucket = table.buckets[slot]; + assert bucket.doc != doc: "bucket.doc=" + bucket.doc + "; doc=" + doc; + bucket.doc = doc; // set doc + bucket.score = scorer.score(); // initialize score + bucket.prohibited = false; + bucket.coord = 1; // initialize coord + table.validSlots[table.validCount++] = slot; + //System.out.println(" validCount=" + table.validCount); + } + } + + // nocommit need random test that covers all of this... + + // First, prohibited + private static final class FirstProhibitedBooleanScorerCollector extends BooleanScorerCollectorBase { + + public FirstProhibitedBooleanScorerCollector(Scorer scorer, BucketTable table) { + super(scorer, table); + } + + @Override + public void collect(final int doc) throws IOException { + // NOTE: do not add to valid slots + final Bucket bucket = table.buckets[doc & BucketTable.MASK]; + assert bucket.doc != doc: "bucket.doc=" + bucket.doc + "; doc=" + doc; + bucket.doc = doc; // set doc + bucket.prohibited = true; + } + } + + // Not first, not prohibited + private static final class BooleanScorerCollector extends BooleanScorerCollectorBase { + + public BooleanScorerCollector(Scorer scorer, BucketTable table) { + super(scorer, table); + } + + @Override + public void collect(final int doc) throws IOException { + final int slot = doc & BucketTable.MASK; + final Bucket bucket = table.buckets[slot]; if (bucket.doc != doc) { // invalid bucket bucket.doc = doc; // set doc bucket.score = scorer.score(); // initialize score - bucket.bits = mask; // initialize mask bucket.coord = 1; // initialize coord - - bucket.next = table.first; // push onto valid list - table.first = bucket; - } else { // valid bucket + bucket.prohibited = false; + table.validSlots[table.validCount++] = slot; + } else if (!bucket.prohibited) { bucket.score += scorer.score(); // increment score - bucket.bits |= mask; // add bits in mask bucket.coord++; // increment coord } } + } + + // Not first, prohibited + private static final class ProhibitedBooleanScorerCollector extends BooleanScorerCollectorBase { - @Override - public void setNextReader(AtomicReaderContext context) { - // not needed by this implementation + public ProhibitedBooleanScorerCollector(Scorer scorer, BucketTable table) { + super(scorer, table); } @Override - public void setScorer(Scorer scorer) { - this.scorer = scorer; + public void collect(final int doc) throws IOException { + final Bucket bucket = table.buckets[doc & BucketTable.MASK]; + if (bucket.doc != doc) { // invalid bucket + bucket.doc = doc; // set doc + bucket.prohibited = true; + bucket.next = table.first; // push onto valid list + table.first = bucket; + // NOTE: do not add to valid slots + } else { + bucket.prohibited = true; + } } - - @Override - public boolean acceptsDocsOutOfOrder() { - return true; - } + } - } - // An internal class which is used in score(Collector, int) for setting the // current score. This is required since Collector exposes a setScorer method // and implementations that need the score will call scorer.score(). @@ -140,10 +206,7 @@ static final class Bucket { int doc = -1; // tells if bucket is valid double score; // incremental score - // TODO: break out bool anyProhibited, int - // numRequiredMatched; then we can remove 32 limit on - // required clauses - int bits; // used for bool constraints + boolean prohibited; // used for NOT constraint int coord; // count of terms in score Bucket next; // next valid bucket } @@ -154,6 +217,9 @@ public static final int MASK = SIZE - 1; final Bucket[] buckets = new Bucket[SIZE]; + final int[] validSlots = new int[SIZE]; + int validCount; + Bucket first = null; // head of valid list public BucketTable() { @@ -164,23 +230,34 @@ } } - public Collector newCollector(int mask) { - return new BooleanScorerCollector(mask, this); + public Collector newCollector(Scorer scorer, boolean isProhibited, boolean isFirst) { + if (isFirst) { + if (isProhibited) { + return new FirstProhibitedBooleanScorerCollector(scorer, this); + } else { + return new FirstBooleanScorerCollector(scorer, this); + } + } else if (isProhibited) { + return new ProhibitedBooleanScorerCollector(scorer, this); + } else { + return new BooleanScorerCollector(scorer, this); + } } - public int size() { return SIZE; } + public int size() { + return SIZE; + } } static final class SubScorer { - public Scorer scorer; + public final Scorer scorer; // TODO: re-enable this if BQ ever sends us required clauses //public boolean required = false; - public boolean prohibited; - public Collector collector; - public SubScorer next; + public final boolean prohibited; + public final Collector collector; public SubScorer(Scorer scorer, boolean required, boolean prohibited, - Collector collector, SubScorer next) { + Collector collector) { if (required) { throw new IllegalArgumentException("this scorer cannot handle required=true"); } @@ -189,110 +266,131 @@ //this.required = required; this.prohibited = prohibited; this.collector = collector; - this.next = next; } } - private SubScorer scorers = null; - private BucketTable bucketTable = new BucketTable(); + private final SubScorer[] scorers; + private final BucketTable bucketTable = new BucketTable(); private final float[] coordFactors; // TODO: re-enable this if BQ ever sends us required clauses //private int requiredMask = 0; private final int minNrShouldMatch; private int end; - private Bucket current; - // Any time a prohibited clause matches we set bit 0: - private static final int PROHIBITED_MASK = 1; + + private static class ScorerAndProhibited implements Comparable { + Scorer scorer; + boolean prohibited; + + public ScorerAndProhibited(Scorer scorer, boolean prohibited) { + this.scorer = scorer; + this.prohibited = prohibited; + } + + @Override + public int compareTo(ScorerAndProhibited other) { + if (prohibited && !other.prohibited) { + // All prohibited clauses first: + return -1; + } else if (!prohibited && other.prohibited) { + // All prohibited clauses first: + return 1; + } else { + // Then, sort by lower docID (= higher hit count, approx): + return scorer.docID() - other.scorer.docID(); + } + } + } BooleanScorer(BooleanWeight weight, boolean disableCoord, int minNrShouldMatch, List optionalScorers, List prohibitedScorers, int maxCoord) throws IOException { super(weight); this.minNrShouldMatch = minNrShouldMatch; + boolean isFirst = true; + List subs = new ArrayList(); + if (optionalScorers != null && optionalScorers.size() > 0) { for (Scorer scorer : optionalScorers) { - if (scorer.nextDoc() != NO_MORE_DOCS) { - scorers = new SubScorer(scorer, false, false, bucketTable.newCollector(0), scorers); + int doc = scorer.nextDoc(); + if (doc != NO_MORE_DOCS) { + subs.add(new ScorerAndProhibited(scorer, false)); } } } if (prohibitedScorers != null && prohibitedScorers.size() > 0) { for (Scorer scorer : prohibitedScorers) { - if (scorer.nextDoc() != NO_MORE_DOCS) { - scorers = new SubScorer(scorer, false, true, bucketTable.newCollector(PROHIBITED_MASK), scorers); + int doc = scorer.nextDoc(); + if (doc != NO_MORE_DOCS) { + subs.add(new ScorerAndProhibited(scorer, true)); } } } + Collections.sort(subs); + + scorers = new SubScorer[subs.size()]; + for(int i=0;i getChildren() { List children = new ArrayList(); - for (SubScorer sub = scorers; sub != null; sub = sub.next) { + for (SubScorer sub : scorers) { // TODO: fix this if BQ ever sends us required clauses children.add(new ChildScorer(sub.scorer, sub.prohibited ? "MUST_NOT" : "SHOULD")); }