Merge pull request #14019 from vespa-engine/arnej/only-taggable-items-may-be-connected

ensure connected query items are Taggable

10 files changed, 97 insertions, 42 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/query/CompositeTaggableItem.java b/container-search/src/main/java/com/yahoo/prelude/query/CompositeTaggableItem.java
index b1912e4128d..dd015b5f040 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/CompositeTaggableItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/CompositeTaggableItem.java
@@ -27,6 +27,10 @@ public abstract class CompositeTaggableItem extends CompositeItem implements Tag
 
     /** See {@link TaggableItem#setConnectivity} */
     public void setConnectivity(Item item, double connectivity) {
+        if (!(item instanceof TaggableItem)) {
+            throw new IllegalArgumentException("setConnectivity item must be taggable, was: "
+                                               + item.getClass() + " [" + item + "]");
+        }
         setHasUniqueID(true);
         item.setHasUniqueID(true);
         if (connectedItem != null) {
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java b/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java
index 6f82f340f4b..afdc859a5b7 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/SimpleTaggableItem.java
@@ -27,6 +27,10 @@ public abstract class SimpleTaggableItem extends Item implements TaggableItem {
 
     @Override
     public void setConnectivity(Item item, double connectivity) {
+        if (!(item instanceof TaggableItem)) {
+            throw new IllegalArgumentException("setConnectivity item must be taggable, was: "
+                                               + item.getClass() + " [" + item + "]");
+        }
         setHasUniqueID(true);
         item.setHasUniqueID(true);
         if (connectedItem != null) {
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/TaggableSegmentItem.java b/container-search/src/main/java/com/yahoo/prelude/query/TaggableSegmentItem.java
index 35df4bf443c..83122a0a512 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/TaggableSegmentItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/TaggableSegmentItem.java
@@ -31,6 +31,10 @@ public abstract class TaggableSegmentItem extends SegmentItem implements Taggabl
 
     /** See {@link TaggableItem#setConnectivity} */
     public void setConnectivity(Item item, double connectivity) {
+        if (!(item instanceof TaggableItem)) {
+            throw new IllegalArgumentException("setConnectivity item must be taggable, was: "
+                                               + item.getClass() + " [" + item + "]");
+        }
         setHasUniqueID(true);
         item.setHasUniqueID(true);
         if (connectedItem != null) {
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java
index 76ea7fb11a8..267880fc97a 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/parser/StructuredParser.java
@@ -667,9 +667,19 @@ abstract class StructuredParser extends AbstractParser {
         int items = composite.items().size();
         if (items < 2) return;
         Item nextToLast = composite.items().get(items - 2);
-        Item last = composite.items().get(items - 1);
+        if (nextToLast instanceof AndSegmentItem) {
+            var subItems = ((AndSegmentItem) nextToLast).items();
+            nextToLast = subItems.get(subItems.size() - 1);
+        }
         if ( ! (nextToLast instanceof TermItem)) return;
-        ((TermItem)nextToLast).setConnectivity(last, 1);
+        Item last = composite.items().get(items - 1);
+        if (last instanceof AndSegmentItem) {
+            last = ((AndSegmentItem) last).items().get(0);
+        }
+        if (last instanceof TaggableItem) {
+            TermItem t1 = (TermItem) nextToLast;
+            t1.setConnectivity(last, 1);
+        }
     }
 
     private boolean addStartMarking() {
diff --git a/container-search/src/test/java/com/yahoo/prelude/query/ItemsCommonStuffTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/ItemsCommonStuffTestCase.java
index 38911ef83c5..a080ae653e5 100644
--- a/container-search/src/test/java/com/yahoo/prelude/query/ItemsCommonStuffTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/query/ItemsCommonStuffTestCase.java
@@ -9,8 +9,6 @@ import java.util.ListIterator;
 import java.util.NoSuchElementException;
 import java.util.regex.PatternSyntaxException;
 
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
 
 import com.yahoo.prelude.query.Item.ItemType;
@@ -18,20 +16,12 @@ import com.yahoo.prelude.query.Item.ItemType;
 /**
  * Check basic contracts common to "many" item implementations.
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 public class ItemsCommonStuffTestCase {
 
-    @Before
-    public void setUp() throws Exception {
-    }
-
-    @After
-    public void tearDown() throws Exception {
-    }
-
     @Test
-    public final void testLoops() {
+    public void testLoops() {
         AndSegmentItem as = new AndSegmentItem("farmyards", false, false);
         boolean caught = false;
         try {
@@ -68,17 +58,17 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testIndexName() {
+    public void testIndexName() {
         WordItem w = new WordItem("nalle");
         AndItem a = new AndItem();
         a.addItem(w);
-        final String expected = "mobil";
+        String expected = "mobil";
         a.setIndexName(expected);
         assertEquals(expected, w.getIndexName());
     }
 
     @Test
-    public final void testBoundaries() {
+    public void testBoundaries() {
         WordItem w = new WordItem("nalle");
         AndItem a = new AndItem();
         boolean caught = false;
@@ -112,7 +102,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testRemoving() {
+    public void testRemoving() {
         AndItem other = new AndItem();
         WordItem w = new WordItem("nalle");
         AndItem a = new AndItem();
@@ -127,7 +117,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testGeneralMutability() {
+    public void testGeneralMutability() {
         AndItem a = new AndItem();
         assertFalse(a.isLocked());
         a.lock();
@@ -135,7 +125,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testCounting() {
+    public void testCounting() {
         WordItem w = new WordItem("nalle");
         AndItem a = new AndItem();
         WordItem v = new WordItem("bamse");
@@ -149,7 +139,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testIteratorJuggling() {
+    public void testIteratorJuggling() {
         AndItem a = new AndItem();
         WordItem w0 = new WordItem("nalle");
         WordItem w1 = new WordItem("bamse");
@@ -178,9 +168,9 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testIdStuff() {
+    public void testIdStuff() {
         Item i;
-        final String expected = "i";
+        String expected = "i";
         i = new ExactStringItem(expected);
         assertEquals(ItemType.EXACT, i.getItemType());
         assertEquals("EXACTSTRING", i.getName());
@@ -206,7 +196,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testEquivBuilding() {
+    public void testEquivBuilding() {
         WordItem w = new WordItem("nalle");
         WordItem v = new WordItem("bamse");
         w.setConnectivity(v, 1.0);
@@ -220,8 +210,8 @@ public class ItemsCommonStuffTestCase {
         WordItem w = new WordItem("nalle");
         WordItem v = new WordItem("bamse");
         w.setConnectivity(v, 1.0);
-        final String expected = "puppy";
-        final String expected2 = "kvalp";
+        String expected = "puppy";
+        String expected2 = "kvalp";
         EquivItem e = new EquivItem(w, Arrays.asList(new String[] { expected, expected2 }));
         assertEquals(1.0, e.getConnectivity(), 1e-9);
         assertSame(v, e.getConnectedItem());
@@ -230,12 +220,12 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testSegment() {
+    public void testSegment() {
         AndSegmentItem as = new AndSegmentItem("farmyards", false, false);
         assertFalse(as.isLocked());
-        final WordItem firstItem = new WordItem("nalle");
+        WordItem firstItem = new WordItem("nalle");
         as.addItem(firstItem);
-        final WordItem item = new WordItem("bamse");
+        WordItem item = new WordItem("bamse");
         as.addItem(1, item);
         assertTrue(as.removeItem(item));
         assertFalse(as.isFromUser());
@@ -266,7 +256,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testMarkersVsWords() {
+    public void testMarkersVsWords() {
         WordItem mw0 = MarkerWordItem.createEndOfHost();
         WordItem mw1 = MarkerWordItem.createStartOfHost();
         WordItem w0 = new WordItem("$");
@@ -279,11 +269,11 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testNumberBasics() {
-        final String expected = "12";
+    public void testNumberBasics() {
+        String expected = "12";
         IntItem i = new IntItem(expected, "num");
         assertEquals(expected, i.stringValue());
-        final String expected2 = "34";
+        String expected2 = "34";
         i.setNumber(expected2);
         assertEquals(expected2, i.stringValue());
         String expected3 = "56";
@@ -297,7 +287,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testNullItemFailsProperly() {
+    public void testNullItemFailsProperly() {
         NullItem n = new NullItem();
         n.setIndexName("nalle");
         boolean caught = false;
@@ -324,7 +314,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testNearisNotAnd() {
+    public void testNearisNotAnd() {
         AndItem a = new AndItem();
         NearItem n = new NearItem();
         n.setDistance(2);
@@ -343,7 +333,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testPhraseSegmentBasics() {
+    public void testPhraseSegmentBasics() {
         AndSegmentItem a = new AndSegmentItem("gnurk", "gurk", false, false);
         fill(a);
         a.lock();
@@ -365,7 +355,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testPhraseConnectivity() {
+    public void testPhraseConnectivity() {
         WordItem w = new WordItem("a");
         PhraseItem p = new PhraseItem();
         fill(p);
@@ -375,7 +365,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testBaseClassPhraseSegments() {
+    public void testBaseClassPhraseSegments() {
         PhraseSegmentItem p = new PhraseSegmentItem("g", false, true);
         fill(p);
         assertEquals(4, p.encode(ByteBuffer.allocate(5000)));
@@ -386,7 +376,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testTermTypeBasic() {
+    public void testTermTypeBasic() {
         assertFalse(TermType.AND.equals(TermType.DEFAULT));
         assertFalse(TermType.AND.equals(Integer.valueOf(10)));
         assertTrue(TermType.AND.equals(TermType.AND));
@@ -397,7 +387,7 @@ public class ItemsCommonStuffTestCase {
     }
 
     @Test
-    public final void testRegexp() {
+    public void testRegexp() {
         RegExpItem empty = new RegExpItem("a", true, "");
         assertTrue(empty.isFromQuery());
         assertTrue(empty.isStemmed());
@@ -416,5 +406,6 @@ public class ItemsCommonStuffTestCase {
         }
         assertEquals("Dangling meta character '*' near index 0\n" + "*\n" + "^", last.getMessage());
     }
+
 }
 
diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java
index 80b2f845e84..f76be321367 100644
--- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParseTestCase.java
@@ -18,7 +18,9 @@ import com.yahoo.prelude.query.PhraseSegmentItem;
 import com.yahoo.prelude.query.PrefixItem;
 import com.yahoo.prelude.query.RankItem;
 import com.yahoo.prelude.query.SubstringItem;
+import com.yahoo.prelude.query.SubstringItem;
 import com.yahoo.prelude.query.SuffixItem;
+import com.yahoo.prelude.query.TaggableItem;
 import com.yahoo.prelude.query.WordItem;
 import com.yahoo.prelude.query.parser.SpecialTokens;
 import com.yahoo.prelude.query.parser.TestLinguistics;
@@ -2508,6 +2510,26 @@ public class ParseTestCase {
     }
 
     @Test
+    public void testAndSegmenting() {
+        Item root = tester.parseQuery("a'b&c'd", Language.ENGLISH, Query.Type.ALL);
+        assertTrue(root instanceof AndItem);
+        AndItem top = (AndItem) root;
+        assertTrue(top.getItem(0) instanceof AndSegmentItem);
+        assertTrue(top.getItem(1) instanceof AndSegmentItem);
+        AndSegmentItem seg1 = (AndSegmentItem) top.getItem(0);
+        AndSegmentItem seg2 = (AndSegmentItem) top.getItem(1);
+        Item t1 = seg1.getItem(0);
+        Item t2 = seg1.getItem(1);
+        Item t3 = seg2.getItem(0);
+        Item t4 = seg2.getItem(1);
+        assertTrue(((TaggableItem)t2).hasUniqueID());
+        assertTrue(((TaggableItem)t3).hasUniqueID());
+        assertTrue(((TaggableItem)t1).getConnectedItem() == t2);
+        assertTrue(((TaggableItem)t2).getConnectedItem() == t3);
+        assertTrue(((TaggableItem)t3).getConnectedItem() == t4);
+    }
+
+    @Test
     public void testSiteAndSegmentPhrases() {
         tester.assertParsed("host.all:\"www abc com x y-z $\"",
                            "host.all:www.abc.com/x'y-z", "",
diff --git a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java
index 9c61b10e15f..17155fff5de 100644
--- a/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java
+++ b/container-search/src/test/java/com/yahoo/prelude/query/parser/test/ParsingTester.java
@@ -124,6 +124,10 @@ public class ParsingTester {
         return root;
     }
 
+    public Item parseQuery(String query, Language language, Query.Type type) {
+        return parseQuery(query, null, language, type, new SimpleLinguistics());
+    }
+
     public Item parseQuery(String query, String filter, Language language, Query.Type type, Linguistics linguistics) {
         Parser parser = ParserFactory.newInstance(type, new ParserEnvironment()
                 .setIndexFacts(indexFacts)
diff --git a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java
index d832ba52ceb..0a502eccc11 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/MinimalQueryInserterTestCase.java
@@ -342,11 +342,18 @@ public class MinimalQueryInserterTestCase {
     }
 
     @Test
+    public void testAndSegmenting() {
+        Query query = new Query("?yql=select%20%2A%20from%20sources%20%2A%20where%20%5B%7B%22defaultIndex%22%3A%20%22default%22%2C%22grammar%22%3A%20%22web%22%2C%22stem%22%3A%20true%2C%22allowEmpty%22%3A%20true%7D%5DuserInput%28%40animal%29%3B&animal=m%26m%27s&tracelevel=3");
+        execution.search(query);
+        assertEquals("select * from sources * where (default contains \"m\" AND default contains ([{\"origin\": {\"original\": \"m\\'s\", \"offset\": 0, \"length\": 3}, \"andSegmenting\": true}]phrase(\"m\", \"s\")));",
+                     query.yqlRepresentation());
+    }
+
+    @Test
     public void verifyThatWarmupIsSane() {
         assertTrue(MinimalQueryInserter.warmup());
     }
 
-
     private static void assertGrouping(String expected, Query query) {
         List<String> actual = new ArrayList<>();
         for (GroupingRequest request : query.getSelect().getGrouping())
diff --git a/container-search/src/test/java/com/yahoo/search/yql/VespaSerializerTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/VespaSerializerTestCase.java
index f8e930fa19d..63840b0f5ec 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/VespaSerializerTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/VespaSerializerTestCase.java
@@ -253,9 +253,13 @@ public class VespaSerializerTestCase {
         andSegment.addItem(new WordItem("a", "indexNamePlaceholder"));
         andSegment.addItem(new WordItem("b", "indexNamePlaceholder"));
         andSegment.setLabel("labeled");
-        andSegment.lock();
         String q = VespaSerializer.serialize(andSegment);
         assertEquals("indexNamePlaceholder contains ([{\"origin\": {\"original\": \"abc\", \"offset\": 0, \"length\": 3}, \"andSegmenting\": true}]phrase(\"a\", \"b\"))", q);
+
+        andSegment.setIndexName("someIndexName");
+        andSegment.lock();
+        q = VespaSerializer.serialize(andSegment);
+        assertEquals("someIndexName contains ([{\"origin\": {\"original\": \"abc\", \"offset\": 0, \"length\": 3}, \"andSegmenting\": true}]phrase(\"a\", \"b\"))", q);
     }
 
     @Test
diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java
index 6e7aa752e29..1b8324c9de8 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java
@@ -1036,6 +1036,11 @@ public class YqlParserTestCase {
         // success: parsed without exception
     }
 
+    @Test
+    public void testAndSegmenting() {
+        parse("select * from sources * where (default contains ([{\"stem\": false}]\"m\") AND default contains ([{\"origin\": {\"original\": \"m\'s\", \"offset\": 0, \"length\": 3}, \"andSegmenting\": true}]phrase(\"m\", \"s\"))) timeout 472;");
+    }
+
     private void assertUrlQuery(String field, Query query, boolean startAnchor, boolean endAnchor, boolean endAnchorIsDefault) {
         boolean startAnchorIsDefault = false; // Always