Handle repeated EQUIV production

5 files changed, 54 insertions, 16 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/semantics/engine/Evaluation.java b/container-search/src/main/java/com/yahoo/prelude/semantics/engine/Evaluation.java
index 07342a48916..989f3040cc7 100644
--- a/container-search/src/main/java/com/yahoo/prelude/semantics/engine/Evaluation.java
+++ b/container-search/src/main/java/com/yahoo/prelude/semantics/engine/Evaluation.java
@@ -244,12 +244,14 @@ public class Evaluation {
      * @param desiredParentType the desired type of the composite which contains item when this returns
      */
     public void insertItem(Item item, CompositeItem parent, int index, TermType desiredParentType) {
-        if (parent == null) { // TODO: Accommodate for termtype in this case too
-            query.getModel().getQueryTree().setRoot(item);
+        if (isEmpty(parent)) {
+            CompositeItem newParent = (CompositeItem)desiredParentType.createItemClass();
+            newParent.addItem(item);
+            query.getModel().getQueryTree().setRoot(newParent);
             return;
         }
 
-        if (parent.getItemCount()>0 && parent instanceof QueryTree && parent.getItem(0) instanceof CompositeItem) {
+        if (parent.getItemCount() > 0 && parent instanceof QueryTree && parent.getItem(0) instanceof CompositeItem) {
             // combine with the existing root instead
             parent = (CompositeItem)parent.getItem(0);
             if (index == 1) { // that means adding it after the existing root
@@ -261,9 +263,23 @@ public class Evaluation {
              && equalIndexNameIfParentIsPhrase(item, parent)) {
             addItem(parent, index, item, desiredParentType);
         }
-        else {
+        else if (incompatible(desiredParentType, parent)) {
             insertIncompatibleItem(item, parent, query, desiredParentType);
         }
+        else {
+            insertIncompatibleItemAsParent(item, parent, query, desiredParentType);
+        }
+    }
+
+    private boolean isEmpty(Item item) {
+        if (item == null) return true;
+        if (item instanceof QueryTree && ((QueryTree) item).isEmpty()) return true;
+        return false;
+    }
+
+    /** Returns true if the desired type cannot have childCandidate as a child */
+    private boolean incompatible(TermType desiredParentType, Item childCandidate) {
+        return desiredParentType == TermType.EQUIV && childCandidate.getItemType() != Item.ItemType.EQUIV;
     }
 
     private void addItem(CompositeItem parent, int index, Item item, TermType desiredParentType) {
@@ -305,6 +321,17 @@ public class Evaluation {
     }
 
     private void insertIncompatibleItem(Item item, CompositeItem parent, Query query, TermType desiredParentType) {
+        CompositeItem newParent;
+        if (desiredParentType == TermType.DEFAULT)
+            newParent = new AndItem();
+        else
+            newParent = (CompositeItem)desiredParentType.createItemClass();
+
+        newParent.addItem(item);
+        parent.addItem(newParent);
+    }
+
+    private void insertIncompatibleItemAsParent(Item item, CompositeItem parent, Query query, TermType desiredParentType) {
         // Create new parent
         CompositeItem newParent;
         if (desiredParentType == TermType.DEFAULT)
@@ -325,11 +352,7 @@ public class Evaluation {
 
         }
         else {
-            int parentIndex = 0;
-            if (parentsParent != null) {
-                parentIndex = parentsParent.getItemIndex(parent);
-            }
-            parentsParent.setItem(parentIndex, newParent);
+            parentsParent.setItem(parentsParent.getItemIndex(parent), newParent);
         }
     }
 
diff --git a/container-search/src/main/java/com/yahoo/prelude/semantics/rule/ReplacingProductionRule.java b/container-search/src/main/java/com/yahoo/prelude/semantics/rule/ReplacingProductionRule.java
index dfa423ec889..70151ea479b 100644
--- a/container-search/src/main/java/com/yahoo/prelude/semantics/rule/ReplacingProductionRule.java
+++ b/container-search/src/main/java/com/yahoo/prelude/semantics/rule/ReplacingProductionRule.java
@@ -14,7 +14,7 @@ public class ReplacingProductionRule extends ProductionRule {
     /** Carries out the production of this rule */
     public void produce(RuleEvaluation e) {
         removeNonreferencedMatches(e);
-        if (e.getTraceLevel()>=5) {
+        if (e.getTraceLevel() >= 5) {
             e.trace(5,"Removed terms to get '" + e.getEvaluation().getQuery().getModel().getQueryTree().getRoot() + "', will add terms");
         }
         super.produce(e);
@@ -22,16 +22,16 @@ public class ReplacingProductionRule extends ProductionRule {
 
     /** Remove items until there's only one item left */
     private void removeNonreferencedMatches(RuleEvaluation e) {
-        int itemCount=e.getEvaluation().getQuerySize();
+        int itemCount = e.getEvaluation().getQuerySize();
 
         // Remove items backwards to ease index handling
-        for (int i=e.getNonreferencedMatchCount()-1; i>=0; i--) {
+        for (int i = e.getNonreferencedMatchCount() - 1; i >= 0; i--) {
             // Ensure we don't produce an empty query
-            if (getProduction().getTermCount()==0 && itemCount==1)
+            if (getProduction().getTermCount() == 0 && itemCount == 1)
                 break;
             itemCount--;
 
-            Match match=e.getNonreferencedMatch(i);
+            Match match = e.getNonreferencedMatch(i);
             match.getItem().getParent().removeItem(match.getPosition());
         }
     }
diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExpansionTestCase.java b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExpansionTestCase.java
index b2d0d60c8fa..fa6b4eefdd5 100644
--- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExpansionTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ExpansionTestCase.java
@@ -15,8 +15,19 @@ public class ExpansionTestCase extends RuleBaseAbstractTestCase {
     }
 
     @Test
-    public void testEquivExpansion() {
+    public void testEquivExpansion1() {
         assertSemantics("EQUIV equiv1 equiv2 equiv3", "equiv1");
     }
 
+    @Test
+    public void testEquivExpansion2() {
+        assertSemantics("EQUIV testfield:e1 testfield:e2 testfield:e3", "testfield:foo");
+    }
+
+    @Test
+    public void testEquivExpansion3() {
+        assertSemantics("AND testfield:e1 testfield:e2 testfield:e3 testfield:e1 testfield:e2 testfield:e3",
+                        "testfield:foo testfield:bar");
+    }
+
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ProductionRuleTestCase.java b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ProductionRuleTestCase.java
index 3513904af02..6c0084d1bdc 100644
--- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/ProductionRuleTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/ProductionRuleTestCase.java
@@ -50,7 +50,7 @@ public class ProductionRuleTestCase {
         RuleEvaluation e = new Evaluation(query).freshRuleEvaluation();
         assertTrue(rule.matches(e));
         rule.produce(e);
-        assertEquals("brand:sony", query.getModel().getQueryTree().getRoot().toString());
+        assertEquals("AND brand:sony", query.getModel().getQueryTree().getRoot().toString());
     }
 
 }
diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/expansion.sr b/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/expansion.sr
index 728c494682b..d03f060cbde 100644
--- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/expansion.sr
+++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/expansion.sr
@@ -2,3 +2,7 @@
 or1 +> ?or2 ?or3;
 
 equiv1 +> =equiv2 =equiv3;
+
+testfield:[test] -> =testfield:e1 =testfield:e2 =testfield:e3;
+
+[test] :- foo, bar, baz;