Collapse nesten compatible weakands

4 files changed, 76 insertions, 44 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java
index 88bae76b26d..2bf20bf7c5a 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/QueryCanonicalizer.java
@@ -94,7 +94,7 @@ public class QueryCanonicalizer {
         if (composite instanceof RankItem || composite instanceof NotItem) {
             collapseLevels(composite, composite.getItemIterator()); // collapse the first item only
         }
-        else if (composite instanceof AndItem || composite instanceof OrItem) {
+        else if (composite instanceof AndItem || composite instanceof OrItem || composite instanceof WeakAndItem) {
             for (ListIterator<Item> i = composite.getItemIterator(); i.hasNext(); )
                 collapseLevels(composite, i);
         }
@@ -106,10 +106,17 @@ public class QueryCanonicalizer {
         Item child = i.next();
         if (child == null) return;
         if (child.getClass() != composite.getClass()) return;
+        if (child instanceof WeakAndItem && !equalWeakAndSettings((WeakAndItem)child, (WeakAndItem)composite)) return;
         i.remove();
         moveChildren((CompositeItem) child, i);
     }
-    
+
+    private static boolean equalWeakAndSettings(WeakAndItem a, WeakAndItem b) {
+        if ( ! a.getIndexName().equals(b.getIndexName())) return false;
+        if (a.getN() != b.getN()) return false;
+        return true;
+    }
+
     private static void moveChildren(CompositeItem from, ListIterator<Item> toIterator) {
         for (ListIterator<Item> i = from.getItemIterator(); i.hasNext(); )
             toIterator.add(i.next());
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WandItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WandItem.java
index 8cce8fb5720..c5679e113f1 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/WandItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/WandItem.java
@@ -99,13 +99,10 @@ public class WandItem extends WeightedSetItem {
     protected void appendHeadingString(StringBuilder buffer) {
         buffer.append(getName());
         buffer.append("(");
-        buffer.append(targetNumHits);
-        buffer.append(",");
-        buffer.append(scoreThreshold);
-        buffer.append(",");
+        buffer.append(targetNumHits).append(",");
+        buffer.append(scoreThreshold).append(",");
         buffer.append(thresholdBoostFactor);
-        buffer.append(")");
-        buffer.append(" ");
+        buffer.append(") ");
     }
 
     @Override
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WeakAndItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WeakAndItem.java
index 4fa2ed8b214..bc335b85611 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/WeakAndItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/WeakAndItem.java
@@ -18,16 +18,20 @@ import java.nio.ByteBuffer;
  */
 public final class WeakAndItem extends NonReducibleCompositeItem {
 
+    /** The default N used if none is specified: 100 */
+    public static final int defaultN = 100;
+
     private int n;
     private String index;
     private int scoreThreshold = 0;
 
-    public ItemType getItemType() {
-        return ItemType.WEAK_AND;
+    /** Creates a WAND item with default N */
+    public WeakAndItem() {
+        this(defaultN);
     }
 
-    public String getName() {
-        return "WAND";
+    public WeakAndItem(int N) {
+        this("", N);
     }
 
     /**
@@ -42,50 +46,37 @@ public final class WeakAndItem extends NonReducibleCompositeItem {
         this.n = n;
         this.index = (index == null) ? "" : index;
     }
-    public WeakAndItem(int N) {
-        this("", N);
-    }
 
-    /** Sets the index name of all subitems of this */
+    @Override
+    public ItemType getItemType() { return ItemType.WEAK_AND; }
+
+    @Override
+    public String getName() { return "WAND"; }
+
+    @Override
     public void setIndexName(String index) {
         String toSet = (index == null) ? "" : index;
         super.setIndexName(toSet);
         this.index = toSet;
     }
 
-    public String getIndexName() {
-        return index;
-    }
+    public String getIndexName() { return index; }
 
     /** Appends the heading of this string - <code>[getName()]([limit]) </code> */
+    @Override
     protected void appendHeadingString(StringBuilder buffer) {
         buffer.append(getName());
         buffer.append("(");
         buffer.append(n);
-        buffer.append(")");
-        buffer.append(" ");
+        buffer.append(") ");
     }
 
-    /** The default N used if none is specified: 100 */
-    public static final int defaultN = 100;
+    public int getN() { return n; }
 
-    /** Creates a WAND item with default N */
-    public WeakAndItem() {
-        this(defaultN);
-    }
-
-    public int getN() {
-        return n;
-    }
-
-    public void setN(int N) {
-        this.n = N;
-    }
+    public void setN(int N) { this.n = N; }
 
     @Deprecated // TODO: Remove on Vespa 8
-    public int getScoreThreshold() {
-        return scoreThreshold;
-    }
+    public int getScoreThreshold() { return scoreThreshold; }
 
     /**
      * Noop.
@@ -93,9 +84,7 @@ public final class WeakAndItem extends NonReducibleCompositeItem {
      * @deprecated has no effect
      */
     @Deprecated // TODO: Remove on Vespa 8
-    public void setScoreThreshold(int scoreThreshold) {
-        this.scoreThreshold = scoreThreshold;
-    }
+    public void setScoreThreshold(int scoreThreshold) { this.scoreThreshold = scoreThreshold; }
 
     @Override
     protected void encodeThis(ByteBuffer buffer) {
@@ -111,9 +100,7 @@ public final class WeakAndItem extends NonReducibleCompositeItem {
     }
 
     @Override
-    public int hashCode() {
-        return super.hashCode() + 31 * n;
-    }
+    public int hashCode() { return super.hashCode() + 31 * n; }
 
     /** Returns whether this item is of the same class and contains the same state as the given item. */
     @Override
diff --git a/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java
index d3b64100c1e..c10e2dcb3ad 100644
--- a/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/query/test/QueryCanonicalizerTestCase.java
@@ -108,6 +108,47 @@ public class QueryCanonicalizerTestCase {
     }
 
     @Test
+    public void testMultilevelWeakAndCollapsing() {
+        CompositeItem root = new WeakAndItem();
+        CompositeItem l1 = new WeakAndItem();
+        CompositeItem l2 = new WeakAndItem();
+        CompositeItem l3 = new WeakAndItem();
+        CompositeItem l4 = new WeakAndItem();
+
+        root.addItem(l1);
+
+        l1.addItem(l2);
+        l1.addItem(new WordItem("l1"));
+
+        l2.addItem(l3);
+        l2.addItem(new WordItem("l2"));
+
+        l3.addItem(l4);
+        l3.addItem(new WordItem("l3"));
+
+        l4.addItem(new WordItem("l4"));
+
+        assertCanonicalized("WAND(100) l4 l3 l2 l1", null, root);
+    }
+
+    @Test
+    public void testWeakAndCollapsingRequireSameNAndIndex() {
+        CompositeItem root = new WeakAndItem(10);
+        CompositeItem l1 = new WeakAndItem(100);
+        CompositeItem l2 = new WeakAndItem(100);
+        l2.setIndexName("other");
+
+        root.addItem(l1);
+
+        l1.addItem(l2);
+        l1.addItem(new WordItem("l1"));
+
+        l2.addItem(new WordItem("l2"));
+
+        assertCanonicalized("WAND(10) (WAND(100) (WAND(100) l2) l1)", null, root);
+    }
+
+    @Test
     public void testNullRoot() {
         assertCanonicalized(null, "No query", new Query());
     }