From f12abad10f1d7deb38f3d64662180852195af15f Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Wed, 21 May 2014 02:15:15 +0000 Subject: [PATCH] LUCENE-4236: try to make more palatable git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/branches/lucene4236@1596440 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/lucene/search/BooleanQuery.java | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java index b483b59d275..e10456e016b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java +++ b/lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java @@ -368,6 +368,14 @@ public class BooleanQuery extends Query implements Iterable { } // scorer simplifications: + + if (optional.size() == minShouldMatch) { + // any optional clauses are in fact required + required.addAll(optional); + optional.clear(); + minShouldMatch = 0; + } + if (required.isEmpty() && optional.isEmpty()) { // no required and optional clauses. return null; @@ -376,14 +384,10 @@ public class BooleanQuery extends Query implements Iterable { // optional scorer. Therefore if there are not enough optional scorers // no documents will be matched by the query return null; - } else if (optional.size() == minShouldMatch) { - // either we have no optional clauses, or they are all required - // nocommit: what if required is empty too? - required.addAll(optional); - optional.clear(); - minShouldMatch = 0; } + // three cases: conjunction, disjunction, or mix + // pure conjunction if (optional.isEmpty()) { return excl(req(required, disableCoord), prohibited); @@ -394,7 +398,12 @@ public class BooleanQuery extends Query implements Iterable { return excl(opt(optional, minShouldMatch, disableCoord), prohibited); } - // conjunction-disjunction mix + // conjunction-disjunction mix: + // we create the required and optional pieces with coord disabled, and then + // combine the two: if minNrShouldMatch > 0, then its a conjunction: because the + // optional side must match. otherwise its required + optional, factoring the + // number of optional terms into the coord calculation + Scorer req = excl(req(required, true), prohibited); Scorer opt = opt(optional, minShouldMatch, true); @@ -439,10 +448,8 @@ public class BooleanQuery extends Query implements Iterable { } } - // nocommit: maybe dont do this optionalCount stuff and just check minNR >= 1 instead? - // we do fancy things in BS2 here anyway if (optionalCount == minNrShouldMatch) { - return false; // BS2 (in-order) will be used, as this means we have mandatory clauses + return false; // BS2 (in-order) will be used, as this means conjunction } // scorer() will return an out-of-order scorer if requested. @@ -450,18 +457,18 @@ public class BooleanQuery extends Query implements Iterable { } private Scorer req(List required, boolean disableCoord) { - Scorer req; if (required.size() == 1) { - req = required.get(0); + Scorer req = required.get(0); if (!disableCoord && maxCoord > 1) { - req = new BoostedScorer(req, coord(required.size(), maxCoord)); + return new BoostedScorer(req, coord(1, maxCoord)); + } else { + return req; } } else { - req = new ConjunctionScorer(this, - required.toArray(new Scorer[required.size()]), - disableCoord ? 1.0F : coord(required.size(), maxCoord)); + return new ConjunctionScorer(this, + required.toArray(new Scorer[required.size()]), + disableCoord ? 1.0F : coord(required.size(), maxCoord)); } - return req; } private Scorer excl(Scorer main, List prohibited) throws IOException { @@ -470,21 +477,23 @@ public class BooleanQuery extends Query implements Iterable { } else if (prohibited.size() == 1) { return new ReqExclScorer(main, prohibited.get(0)); } else { - // TODO: this scores the required clauses (which is stupid). but we always did this. float coords[] = new float[prohibited.size()+1]; Arrays.fill(coords, 1F); - return new ReqExclScorer(main, new DisjunctionSumScorer(this, - prohibited.toArray(new Scorer[prohibited.size()]), - coords)); + // TODO: don't score here. + return new ReqExclScorer(main, + new DisjunctionSumScorer(this, + prohibited.toArray(new Scorer[prohibited.size()]), + coords)); } } private Scorer opt(List optional, int minShouldMatch, boolean disableCoord) throws IOException { - Scorer opt; if (optional.size() == 1) { - opt = optional.get(0); + Scorer opt = optional.get(0); if (!disableCoord && maxCoord > 1) { - opt = new BoostedScorer(opt, coord(optional.size(), maxCoord)); + return new BoostedScorer(opt, coord(1, maxCoord)); + } else { + return opt; } } else { float coords[]; @@ -496,12 +505,12 @@ public class BooleanQuery extends Query implements Iterable { } if (minShouldMatch > 1) { return new MinShouldMatchSumScorer(this, optional, minShouldMatch, coords); + } else { + return new DisjunctionSumScorer(this, + optional.toArray(new Scorer[optional.size()]), + coords); } - opt = new DisjunctionSumScorer(this, - optional.toArray(new Scorer[optional.size()]), - coords); } - return opt; } private float[] coords() {