You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2017/04/20 16:29:41 UTC

[08/13] lucene-solr:jira/solr-8668: SOLR-10527: edismax with sow=false fails to create dismax-per-term queries when any field is boosted

SOLR-10527: edismax with sow=false fails to create dismax-per-term queries when any field is boosted


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/8b27b82b
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/8b27b82b
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/8b27b82b

Branch: refs/heads/jira/solr-8668
Commit: 8b27b82babed40e72eceaec0765d448247a7d322
Parents: 6eac7fa
Author: Steve Rowe <sa...@gmail.com>
Authored: Wed Apr 19 19:02:32 2017 -0400
Committer: Steve Rowe <sa...@gmail.com>
Committed: Wed Apr 19 19:03:31 2017 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  3 ++
 .../solr/search/ExtendedDismaxQParser.java      | 55 +++++++++++++++++---
 .../solr/search/TestExtendedDismaxParser.java   | 23 ++++++++
 3 files changed, 73 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b27b82b/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 2b82024..73fb88a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -212,6 +212,9 @@ Bug Fixes
 
 * SOLR-10047: Mismatched Docvalues segments cause exception in Sorting/Faceting. Solr now uninverts per segment
   to avoid such exceptions. (Keith Laban via shalin)
+  
+* SOLR-10527: edismax with sow=false fails to create dismax-per-term queries when any field is boosted.
+  (Steve Rowe)
 
 * SOLR-10472: Fixed uninversion (aka: FieldCache) bugs with the numeric PointField classes, and CurrencyField (hossman)
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b27b82b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
index 07d7e51..937f77e 100644
--- a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
+++ b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
@@ -1063,7 +1063,7 @@ public class ExtendedDismaxQParser extends QParser {
       this.val = null;
       this.vals = queryTerms;
       this.slop = getPhraseSlop();
-      return getAliasedMultiTermQuery(queryTerms);
+      return getAliasedMultiTermQuery();
     }
 
     @Override
@@ -1188,11 +1188,11 @@ public class ExtendedDismaxQParser extends QParser {
      * DisjunctionMaxQuery.  (so yes: aliases which point at other
      * aliases should work)
      */
-    protected Query getAliasedMultiTermQuery(List<String> queryTerms) throws SyntaxError {
+    protected Query getAliasedMultiTermQuery() throws SyntaxError {
       Alias a = aliases.get(field);
       this.validateCyclicAliasing(field);
       if (a != null) {
-        List<Query> lst = getQueries(a);
+        List<Query> lst = getMultiTermQueries(a);
         if (lst == null || lst.size() == 0) {
           return getQuery();
         }
@@ -1203,14 +1203,25 @@ public class ExtendedDismaxQParser extends QParser {
         // DisMaxQuery.rewrite() removes itself if there is just a single clause anyway.
         // if (lst.size()==1) return lst.get(0);
         if (makeDismax) {
-          if (lst.get(0) instanceof BooleanQuery && allSameQueryStructure(lst)) {
+          Query firstQuery = lst.get(0);
+          if ((firstQuery instanceof BooleanQuery
+              || (firstQuery instanceof BoostQuery && ((BoostQuery)firstQuery).getQuery() instanceof BooleanQuery))
+              && allSameQueryStructure(lst)) {
             BooleanQuery.Builder q = new BooleanQuery.Builder();
             List<Query> subs = new ArrayList<>(lst.size());
-            for (int c = 0 ; c < ((BooleanQuery)lst.get(0)).clauses().size() ; ++c) {
+            BooleanQuery firstBooleanQuery = firstQuery instanceof BoostQuery
+                ? (BooleanQuery)((BoostQuery)firstQuery).getQuery() : (BooleanQuery)firstQuery;
+            for (int c = 0 ; c < firstBooleanQuery.clauses().size() ; ++c) {
               subs.clear();
               // Make a dismax query for each clause position in the boolean per-field queries.
               for (int n = 0 ; n < lst.size() ; ++n) {
-                subs.add(((BooleanQuery)lst.get(n)).clauses().get(c).getQuery());
+                if (lst.get(n) instanceof BoostQuery) {
+                  BoostQuery boostQuery = (BoostQuery)lst.get(n);
+                  BooleanQuery booleanQuery = (BooleanQuery)boostQuery.getQuery();
+                  subs.add(new BoostQuery(booleanQuery.clauses().get(c).getQuery(), boostQuery.getBoost()));
+                } else {
+                  subs.add(((BooleanQuery)lst.get(n)).clauses().get(c).getQuery());
+                }
               }
               q.add(newBooleanClause(new DisjunctionMaxQuery(subs, a.tie), BooleanClause.Occur.SHOULD));
             }
@@ -1239,12 +1250,21 @@ public class ExtendedDismaxQParser extends QParser {
       }
     }
 
-    /** Recursively examines the given query list for identical structure in all queries. */
+    /**
+     * Recursively examines the given query list for identical structure in all queries.
+     * Boosts on BoostQuery-s are ignored, and the contained queries are instead used as the basis for comparison.
+     **/
     private boolean allSameQueryStructure(List<Query> lst) {
       boolean allSame = true;
       Query firstQuery = lst.get(0);
+      if (firstQuery instanceof BoostQuery) {
+        firstQuery = ((BoostQuery)firstQuery).getQuery(); // ignore boost; compare contained query
+      }
       for (int n = 1 ; n < lst.size(); ++n) {
         Query nthQuery = lst.get(n);
+        if (nthQuery instanceof BoostQuery) {
+          nthQuery = ((BoostQuery)nthQuery).getQuery();
+        }
         if (nthQuery.getClass() != firstQuery.getClass()) {
           allSame = false;
           break;
@@ -1350,7 +1370,26 @@ public class ExtendedDismaxQParser extends QParser {
       }
       return lst;
     }
-    
+
+    protected List<Query> getMultiTermQueries(Alias a) throws SyntaxError {
+      if (a == null) return null;
+      if (a.fields.size()==0) return null;
+      List<Query> lst= new ArrayList<>(4);
+
+      for (String f : a.fields.keySet()) {
+        this.field = f;
+        Query sub = getAliasedMultiTermQuery();
+        if (sub != null) {
+          Float boost = a.fields.get(f);
+          if (boost != null && boost.floatValue() != 1f) {
+            sub = new BoostQuery(sub, boost);
+          }
+          lst.add(sub);
+        }
+      }
+      return lst;
+    }
+
     private Query getQuery() {
       try {
         

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8b27b82b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
index 6c7b0cf..61c9a65 100644
--- a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
@@ -1820,6 +1820,29 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
       }
     }
   }
+  
+  public void testSowFalseWithBoost() throws Exception {
+    try (SolrQueryRequest req = req("sow", "false", "qf", "subject title")) {
+      QParser qParser = QParser.getParser("one two", "edismax", req);
+      Query q = qParser.getQuery();
+      assertEquals("+((title:one | subject:on) (title:two | subject:two))", q.toString());
+    }
+    try (SolrQueryRequest req = req("sow", "false", "qf", "subject title^5")) {
+      QParser qParser = QParser.getParser("one two", "edismax", req);
+      Query q = qParser.getQuery();
+      assertEquals("+(((title:one)^5.0 | subject:on) ((title:two)^5.0 | subject:two))", q.toString());
+    }
+    try (SolrQueryRequest req = req("sow", "false", "qf", "subject^3 title")) {
+      QParser qParser = QParser.getParser("one two", "edismax", req);
+      Query q = qParser.getQuery();
+      assertEquals("+((title:one | (subject:on)^3.0) (title:two | (subject:two)^3.0))", q.toString());
+    }
+    try (SolrQueryRequest req = req("sow", "false", "qf", "subject^10 title^20")) {
+      QParser qParser = QParser.getParser("one two", "edismax", req);
+      Query q = qParser.getQuery();
+      assertEquals("+(((title:one)^20.0 | (subject:on)^10.0) ((title:two)^20.0 | (subject:two)^10.0))", q.toString());
+    }
+  }
 
 
   private boolean containsClause(Query query, String field, String value,