Merge pull request #20665 from vespa-engine/bratseth/termlist

Support an external list of terms in term list operators

9 files changed, 342 insertions, 10 deletions

diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json
index 6ed01c2a998..43ce1578e04 100644
--- a/container-search/abi-spec.json
+++ b/container-search/abi-spec.json
@@ -1740,7 +1740,7 @@
       "public java.lang.Integer addToken(long, int)",
       "public java.lang.Integer addToken(java.lang.String, int)",
       "public java.lang.Integer addToken(java.lang.String)",
-      "public java.lang.Integer getTokenWeight(java.lang.String)",
+      "public java.lang.Integer getTokenWeight(java.lang.Object)",
       "public java.lang.Integer removeToken(java.lang.String)",
       "public int getNumTokens()",
       "public java.util.Iterator getTokens()",
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 4770a02e51a..f71f25821ad 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
@@ -72,4 +72,5 @@ public abstract class SimpleTaggableItem extends Item implements TaggableItem {
     public boolean hasUniqueID() {
         return super.hasUniqueID();
     }
+
 }
diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
index e75a8417328..a988753d699 100644
--- a/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
+++ b/container-search/src/main/java/com/yahoo/prelude/query/WeightedSetItem.java
@@ -22,7 +22,6 @@ import java.util.Objects;
  * contain the weights of all the matched tokens in descending
  * order. Each matched weight will be represented as a standard
  * occurrence on position 0 in element 0.
- *
  */
 public class WeightedSetItem extends SimpleTaggableItem {
 
@@ -79,7 +78,7 @@ public class WeightedSetItem extends SimpleTaggableItem {
         return addToken(token, 1);
     }
 
-    public Integer getTokenWeight(String token) {
+    public Integer getTokenWeight(Object token) {
         return set.get(token);
     }
 
diff --git a/container-search/src/main/java/com/yahoo/search/yql/ParameterListParser.java b/container-search/src/main/java/com/yahoo/search/yql/ParameterListParser.java
new file mode 100644
index 00000000000..5a609f0025b
--- /dev/null
+++ b/container-search/src/main/java/com/yahoo/search/yql/ParameterListParser.java
@@ -0,0 +1,204 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.yql;
+
+import com.yahoo.prelude.query.WeightedSetItem;
+
+import java.util.Arrays;
+
+/**
+ * Parser of parameter lists on the form {key:value, key:value} or [[key,value], [key,value], ...]
+ *
+ * @author bratseth
+ */
+class ParameterListParser {
+
+    public static void addItemsFromString(String string, WeightedSetItem out) {
+        var s = new ParsableString(string);
+        switch (s.peek()) {
+            case '[' : addArrayItems(s, out); break;
+            case '{' : addMapItems(s, out); break;
+            default : throw new IllegalArgumentException("Expected a string starting by '[' or '{', " +
+                                                         "but was '" + s.peek() + "'");
+        }
+    }
+
+    private static void addArrayItems(ParsableString s, WeightedSetItem out) {
+        s.pass('[');
+        while (s.peek() != ']') {
+            s.pass('[');
+            long key = s.longTo(s.position(','));
+            s.pass(',');
+            int value = s.intTo(s.position(']'));
+            s.pass(']');
+            out.addToken(key, value);
+            s.passOptional(',');
+            if (s.atEnd()) throw new IllegalArgumentException("Expected an array ending by ']'");
+        }
+        s.pass(']');
+    }
+
+    private static void addMapItems(ParsableString s, WeightedSetItem out) {
+        s.pass('{');
+        while (s.peek() != '}') {
+            String key;
+            if (s.passOptional('\'')) {
+                key = s.stringTo(s.position('\''));
+                s.pass('\'');
+            }
+            else if (s.passOptional('"')) {
+                key = s.stringTo(s.position('"'));
+                s.pass('"');
+            }
+            else {
+                key = s.stringTo(s.position(':')).trim();
+            }
+            s.pass(':');
+            int value = s.intTo(s.position(',','}'));
+            out.addToken(key, value);
+            s.passOptional(',');
+            if (s.atEnd()) throw new IllegalArgumentException("Expected a map ending by '}'");
+        }
+        s.pass('}');
+    }
+
+    private static class ParsableString {
+
+        int position = 0;
+        String s;
+
+        ParsableString(String s) {
+            this.s = s;
+        }
+
+        /**
+         * Returns the next non-space character or UNASSIGNED if we have reached the end of the string.
+         * The current position is not changed.
+         */
+        char peek() {
+            int localPosition = position;
+            while (localPosition < s.length()) {
+                char nextChar = s.charAt(localPosition++);
+                if (!Character.isSpaceChar(nextChar))
+                    return nextChar;
+            }
+            return Character.UNASSIGNED;
+        }
+
+        /**
+         * Verifies that the next non-space character is the given and moves the position past it.
+         *
+         * @throws IllegalArgumentException if the next non-space character is not the given character
+         */
+        void pass(char character) {
+            while (position < s.length()) {
+                char nextChar = s.charAt(position++);
+                if (!Character.isSpaceChar(nextChar)) {
+                    if (nextChar == character)
+                        return;
+                    else
+                        throw new IllegalArgumentException("Expected '" + character + "' at position " + (position-1) +
+                                                           " but got '" + nextChar + "'");
+                }
+            }
+            throw new IllegalArgumentException("Expected '" + character + "' at position " + (position-1) +
+                                               " but reached the end");
+        }
+
+        /**
+         * Checks if the next non-space character is the given and moves the position past it if so.
+         * Does not change the position otherwise.
+         *
+         * @return true if the next non-space character was the given character
+         */
+        boolean passOptional(char character) {
+            int localPosition = position;
+            while (localPosition < s.length()) {
+                char nextChar = s.charAt(localPosition++);
+                if (!Character.isSpaceChar(nextChar)) {
+                    if (nextChar == character) {
+                        position = localPosition;
+                        return true;
+                    } else {
+                        return false;
+                    }
+                }
+            }
+            return false;
+        }
+
+        /**
+         * Returns the position of the next occurrence of any of the given characters.
+         *
+         * @throws IllegalArgumentException if there are no further occurrences of any of the given characters
+         */
+        int position(char ... characters) {
+            int localPosition = position;
+            while (localPosition < s.length()) {
+                char nextChar = s.charAt(localPosition);
+                for (char character : characters)
+                    if (nextChar == character) return localPosition;
+                localPosition++;
+            }
+            throw new IllegalArgumentException("Expected one of " + Arrays.toString(characters) + " after " + position);
+        }
+
+        boolean atEnd() {
+            return position >= s.length();
+        }
+
+        /**
+         * Returns the string value from the current to the given position, and moves the current
+         * position to the next character.
+         *
+         * @throws IllegalArgumentException if end is beyond the last position of the string
+         */
+        String stringTo(int end) {
+            try {
+                String value = s.substring(position, end);
+                position = end;
+                return value;
+            }
+            catch (IndexOutOfBoundsException e) {
+                throw new IllegalArgumentException(end + " is larger than the size of the string,  " + s.length());
+            }
+        }
+
+        /**
+         * Returns the int value from the current to the given position, and moves the current
+         * position to the next character.
+         *
+         * @throws IllegalArgumentException if the string cannot be parsed to an int or end is larger than the string
+         */
+        int intTo(int end) {
+            int start = position;
+            String value = stringTo(end);
+            try {
+                return Integer.parseInt(value.trim());
+            }
+            catch (NumberFormatException e) {
+                throw new IllegalArgumentException("Expected an integer between positions " + start + " and " + end +
+                                                   ", but got " + value);
+            }
+        }
+
+        /**
+         * Returns the long value from the current to the given position, and moves the current
+         * position to the next character.
+         *
+         * @throws IllegalArgumentException if the string cannot be parsed to a long or end is larger than the string
+         */
+        long longTo(int end) {
+            int start = position;
+            String value = stringTo(end);
+            try {
+                return Long.parseLong(value.trim());
+            }
+            catch (NumberFormatException e) {
+                throw new IllegalArgumentException("Expected an integer between positions " + start + " and " + end +
+                                                   ", but got " + value);
+            }
+        }
+
+    }
+
+}
diff --git a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
index 199cf7bb2a9..c0c5b0ee0b0 100644
--- a/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
+++ b/container-search/src/main/java/com/yahoo/search/yql/YqlParser.java
@@ -1669,7 +1669,7 @@ public class YqlParser implements Parser {
                 "Expected operator READ_FIELD or PRPPREF, got %s.", ast.getOperator());
     }
 
-    private static void addItems(OperatorNode<ExpressionOperator> ast, WeightedSetItem out) {
+    private void addItems(OperatorNode<ExpressionOperator> ast, WeightedSetItem out) {
         switch (ast.getOperator()) {
             case MAP:
                 addStringItems(ast, out);
@@ -1677,6 +1677,10 @@ public class YqlParser implements Parser {
             case ARRAY:
                 addLongItems(ast, out);
                 break;
+            case VARREF:
+                Preconditions.checkState(userQuery != null, "Query properties are not available");
+                ParameterListParser.addItemsFromString(userQuery.properties().getString(ast.getArgument(0, String.class)), out);
+                break;
             default:
                 throw newUnexpectedArgumentException(ast.getOperator(),
                                                      ExpressionOperator.ARRAY, ExpressionOperator.MAP);
@@ -1704,10 +1708,8 @@ public class YqlParser implements Parser {
             OperatorNode<ExpressionOperator> tokenValueNode = args.get(0);
             assertHasOperator(tokenValueNode, ExpressionOperator.LITERAL);
             Number tokenValue = tokenValueNode.getArgument(0, Number.class);
-            Preconditions.checkArgument(tokenValue instanceof Integer
-                    || tokenValue instanceof Long,
-                    "Expected Integer or Long, got %s.", tokenValue.getClass()
-                            .getName());
+            Preconditions.checkArgument(tokenValue instanceof Integer || tokenValue instanceof Long,
+                    "Expected Integer or Long, got %s.", tokenValue.getClass().getName());
 
             OperatorNode<ExpressionOperator> tokenWeightNode = args.get(1);
             assertHasOperator(tokenWeightNode, ExpressionOperator.LITERAL);
diff --git a/container-search/src/test/java/com/yahoo/search/yql/ParameterListParserTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/ParameterListParserTestCase.java
new file mode 100644
index 00000000000..44f784e96f3
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/yql/ParameterListParserTestCase.java
@@ -0,0 +1,47 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.yql;
+
+import com.yahoo.prelude.query.WeightedSetItem;
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author bratseth
+ */
+public class ParameterListParserTestCase {
+
+    @Test
+    public void testMapParsing() {
+        assertParsed("{}", Map.of());
+        assertParsed("{a:12}", Map.of("a", 12));
+        assertParsed("{'a':12}", Map.of("a", 12));
+        assertParsed("{\"a\":12}", Map.of("a", 12));
+        assertParsed("{a:12,b:13}", Map.of("a", 12, "b", 13));
+        assertParsed("{a:12, b:13}", Map.of("a", 12, "b", 13));
+        assertParsed("  { a:12, b:13} ", Map.of("a", 12, "b", 13));
+        assertParsed("{a:12, 'b':13} ", Map.of("a", 12, "b", 13));
+        assertParsed("{a:12,'b':13, \"c,}\": 14}", Map.of("a", 12, "b", 13, "c,}", 14));
+    }
+
+    @Test
+    public void testArrayParsing() {
+        assertParsed("[]", Map.of());
+        assertParsed("[[0,12]]", Map.of(0L, 12));
+        assertParsed("[[0,12],[1,13]]", Map.of(0L, 12, 1L, 13));
+        assertParsed("[[0,12], [1,13]]", Map.of(0L, 12, 1L, 13));
+        assertParsed("  [ [0,12], [ 1,13]] ", Map.of(0L, 12, 1L, 13));
+    }
+
+    private void assertParsed(String string, Map<?, Integer> expected) {
+        WeightedSetItem item = new WeightedSetItem("test");
+        ParameterListParser.addItemsFromString(string, item);
+        for (var entry : expected.entrySet()) {
+            assertEquals("Key '" + entry.getKey() + "'", entry.getValue(), item.getTokenWeight(entry.getKey()));
+        }
+        assertEquals("Token count is correct", expected.size(), item.getNumTokens());
+    }
+
+}
diff --git a/container-search/src/test/java/com/yahoo/search/yql/TermListTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/TermListTestCase.java
new file mode 100644
index 00000000000..ac0d676caf7
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/yql/TermListTestCase.java
@@ -0,0 +1,78 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.yql;
+
+import com.yahoo.component.chain.Chain;
+import com.yahoo.search.Query;
+import com.yahoo.search.Result;
+import com.yahoo.search.searchchain.Execution;
+import org.apache.http.client.utils.URIBuilder;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * Tests YQL expressions where a list of terms are supplied by indirection
+ *
+ * @author bratseth
+ */
+public class TermListTestCase {
+
+    @Test
+    public void testTermListInWeightedSet() {
+        URIBuilder builder = searchUri();
+        builder.setParameter("myTerms", "{'1':1, '2':1, '3':1}");
+        builder.setParameter("yql", "select * from sources * where weightedSet(user_id, @myTerms)");
+        Query query = searchAndAssertNoErrors(builder);
+        assertEquals("select * from sources * where weightedSet(user_id, {\"1\": 1, \"2\": 1, \"3\": 1});",
+                     query.yqlRepresentation());
+    }
+
+    @Test
+    public void testTermListInWand() {
+        URIBuilder builder = searchUri();
+        builder.setParameter("myTerms", "{'1':1, '2':1, '3':1}");
+        builder.setParameter("yql", "select * from sources * where wand(user_id, @myTerms)");
+        Query query = searchAndAssertNoErrors(builder);
+        assertEquals("select * from sources * where wand(user_id, {\"1\": 1, \"2\": 1, \"3\": 1});",
+                     query.yqlRepresentation());
+    }
+
+    @Test
+    public void testTermListInDotProduct() {
+        URIBuilder builder = searchUri();
+        builder.setParameter("myTerms", "{'1':1, '2':1, '3':1}");
+        builder.setParameter("yql", "select * from sources * where dotProduct(user_id, @myTerms)");
+        Query query = searchAndAssertNoErrors(builder);
+        assertEquals("select * from sources * where dotProduct(user_id, {\"1\": 1, \"2\": 1, \"3\": 1});",
+                     query.yqlRepresentation());
+    }
+
+    private Query searchAndAssertNoErrors(URIBuilder builder) {
+        Query query = new Query(builder.toString());
+        var searchChain = new Chain<>(new MinimalQueryInserter());
+        var context = Execution.Context.createContextStub();
+        var execution = new Execution(searchChain, context);
+        Result r = execution.search(query);
+        var exception = exceptionOf(r);
+        assertNull(exception == null ? "No error":
+                   exception.getMessage() + "\n" + Arrays.toString(exception.getStackTrace()),
+                   r.hits().getError());
+        return query;
+    }
+
+    private Throwable exceptionOf(Result r) {
+        if (r.hits().getError() == null) return null;
+        if (r.hits().getError().getCause() == null) return null;
+        return r.hits().getError().getCause();
+    }
+
+    private URIBuilder searchUri() {
+        URIBuilder builder = new URIBuilder();
+        builder.setPath("search/");
+        return builder;
+    }
+
+}
diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
index 27959948536..5d3a95efc78 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
@@ -27,7 +27,7 @@ import com.yahoo.search.searchchain.testutil.DocumentSourceSearcher;
 import static com.yahoo.search.searchchain.testutil.DocumentSourceSearcher.DEFAULT_SUMMARY_CLASS;;
 
 /**
- * Test translation of fields and sources in YQL+ to the associated concepts in Vespa.
+ * Test translation of fields and sources in YQL to the associated concepts in Vespa.
  */
 public class YqlFieldAndSourceTestCase {
 
@@ -40,7 +40,6 @@ public class YqlFieldAndSourceTestCase {
     private Execution.Context context;
     private Execution execution;
 
-
     @Before
     public void setUp() throws Exception {
         Query query = new Query("?query=test");
@@ -137,6 +136,7 @@ public class YqlFieldAndSourceTestCase {
         assertFalse(result.hits().get(0).isFilled(DEFAULT_SUMMARY_CLASS));
         assertFalse(result.hits().get(0).isFilled(Execution.ATTRIBUTEPREFETCH));
     }
+
     @Test
     public final void testTrivialCaseWithOnlyDiskfieldWrongClassRequested() {
         final Query query = new Query("?query=test&presentation.summaryFields=" + FIELD1);
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 881101a7bda..636619bf1cc 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
@@ -1253,4 +1253,5 @@ public class YqlParserTestCase {
             actual.add(step.continuations().toString() + step.getOperation());
         return actual.toString();
     }
+
 }