Don't remove OR terms which contributes to rank

2 files changed, 40 insertions, 48 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
index be61ca709d1..61c41c7dc45 100644
--- a/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
+++ b/container-search/src/main/java/com/yahoo/prelude/querytransform/QueryRewrite.java
@@ -41,8 +41,6 @@ public class QueryRewrite {
 
     /**
      * Optimizes the given query tree based on its {@link Model#getRestrict()} parameter, if any.
-     *
-     * @param query to optimize.
      */
     public static void optimizeByRestrict(Query query) {
         if (query.getModel().getRestrict().size() != 1) {
@@ -56,8 +54,6 @@ public class QueryRewrite {
 
     /**
      * Collapses all single-child {@link CompositeItem}s into their parent item.
-     *
-     * @param query The query whose composites to collapse.
      */
     public static void collapseSingleComposites(Query query) {
         Item oldRoot = query.getModel().getQueryTree().getRoot();
@@ -70,8 +66,6 @@ public class QueryRewrite {
     /**
      * Replaces and {@link SimpleIndexedItem} searching in the {@link Hit#SDDOCNAME_FIELD} with an item
      * appropriate for the search node.
-     *
-     * @param query the query to rewrite.
      */
     public static void rewriteSddocname(Query query) {
         Item oldRoot = query.getModel().getQueryTree().getRoot();
@@ -156,53 +150,45 @@ public class QueryRewrite {
         for (int i = item.getItemCount(); --i >= 1; ) {
             Item child = item.getItem(i);
             switch (optimizeByRestrict(child, restrictParam)) {
-            case RECALLS_EVERYTHING:
-                return Recall.RECALLS_NOTHING;
-            case RECALLS_NOTHING:
-                item.removeItem(i);
-                break;
+                case RECALLS_EVERYTHING:
+                    return Recall.RECALLS_NOTHING;
+                case RECALLS_NOTHING:
+                    item.removeItem(i);
+                    break;
             }
         }
         return Recall.UNKNOWN_RECALL;
     }
 
     private static Recall optimizeCompositeItemByRestrict(CompositeItem item, String restrictParam) {
+        Recall recall = Recall.UNKNOWN_RECALL;
         for (int i = item.getItemCount(); --i >= 0; ) {
             switch (optimizeByRestrict(item.getItem(i), restrictParam)) {
-            case RECALLS_EVERYTHING:
-                if ((item instanceof OrItem) || (item instanceof EquivItem)) {
-                    retainChild(item, i);
-                    return Recall.RECALLS_EVERYTHING;
-                } else if ((item instanceof AndItem) || (item instanceof NearItem)) {
-                    item.removeItem(i);
-                } else if (item instanceof RankItem) {
-                    // empty
-                } else {
-                    throw new UnsupportedOperationException(item.getClass().getName());
-                }
-                break;
-            case RECALLS_NOTHING:
-                if ((item instanceof OrItem) || (item instanceof EquivItem)) {
-                    item.removeItem(i);
-                } else if ((item instanceof AndItem) || (item instanceof NearItem)) {
-                    return Recall.RECALLS_NOTHING;
-                } else if (item instanceof RankItem) {
-                    item.removeItem(i);
-                } else {
-                    throw new UnsupportedOperationException(item.getClass().getName());
-                }
-                break;
+                case RECALLS_EVERYTHING:
+                    if ((item instanceof OrItem) || (item instanceof EquivItem)) {
+                        recall = Recall.RECALLS_EVERYTHING;
+                    } else if ((item instanceof AndItem) || (item instanceof NearItem)) {
+                        item.removeItem(i);
+                    } else if (item instanceof RankItem) {
+                        // empty
+                    } else {
+                        throw new UnsupportedOperationException(item.getClass().getName());
+                    }
+                    break;
+                case RECALLS_NOTHING:
+                    if ((item instanceof OrItem) || (item instanceof EquivItem)) {
+                        item.removeItem(i);
+                    } else if ((item instanceof AndItem) || (item instanceof NearItem)) {
+                        return Recall.RECALLS_NOTHING;
+                    } else if (item instanceof RankItem) {
+                        item.removeItem(i);
+                    } else {
+                        throw new UnsupportedOperationException(item.getClass().getName());
+                    }
+                    break;
             }
         }
-        return Recall.UNKNOWN_RECALL;
-    }
-
-    private static void retainChild(CompositeItem item, int childIdx) {
-        Item child = item.removeItem(childIdx);
-        for (int i = item.getItemCount(); --i >= 0; ) {
-            item.removeItem(i);
-        }
-        item.addItem(child);
+        return recall;
     }
 
     private static Item collapseSingleComposites(Item item) {
diff --git a/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java b/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java
index d11fc15fe90..7322b3da6b8 100644
--- a/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/querytransform/test/QueryRewriteTestCase.java
@@ -17,14 +17,14 @@ import static org.junit.Assert.assertTrue;
 public class QueryRewriteTestCase {
 
     @Test
-    public void requireThatOptimizeByRestrictSimplifiesORItemsThatHaveFullRecall() {
+    public void requireThatOptimizeByRestrictSimplifiesORItemsThatHaveFullRecallAndDontImpactRank() {
         assertRewritten("sddocname:foo OR sddocname:bar OR sddocname:baz", "foo", "sddocname:foo");
         assertRewritten("sddocname:foo OR sddocname:bar OR sddocname:baz", "bar", "sddocname:bar");
         assertRewritten("sddocname:foo OR sddocname:bar OR sddocname:baz", "baz", "sddocname:baz");
 
-        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "foo", "sddocname:foo");
-        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "bar", "sddocname:bar");
-        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "baz", "sddocname:baz");
+        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "foo", "OR lhs sddocname:foo");
+        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "bar", "OR lhs sddocname:bar");
+        assertRewritten("lhs OR (sddocname:foo OR sddocname:bar OR sddocname:baz)", "baz", "OR lhs sddocname:baz");
 
         assertRewritten("lhs AND (sddocname:foo OR sddocname:bar OR sddocname:baz)", "foo", "lhs");
         assertRewritten("lhs AND (sddocname:foo OR sddocname:bar OR sddocname:baz)", "bar", "lhs");
@@ -32,6 +32,12 @@ public class QueryRewriteTestCase {
     }
 
     @Test
+    public void testRestrictRewriteDoesNotRemoveRankContributingTerms() {
+        assertRewritten("sddocname:per OR foo OR bar", "per", "OR sddocname:per foo bar");
+        assertRewritten("sddocname:per OR foo OR (bar AND fuz)", "per", "OR sddocname:per foo (AND bar fuz)");
+    }
+
+    @Test
     public void requireThatOptimizeByRestrictSimplifiesANDItemsThatHaveZeroRecall() {
         assertRewritten("sddocname:foo AND bar AND baz", "cox", "NULL");
         assertRewritten("foo AND sddocname:bar AND baz", "cox", "NULL");
@@ -59,7 +65,7 @@ public class QueryRewriteTestCase {
         assertRewritten("sddocname:perder ANDNOT b", "per", "NULL");
         assertRewritten("a ANDNOT sddocname:per a b", "per", "NULL");
     }
-
+    
     @Test
     public void testRestrictRank() {
         assertRewritten("sddocname:per&filter=abc", "espen", "|abc");