Merge pull request #21787 from vespa-engine/bjorncs/grouping-max-infv7.564.19

Bjorncs/grouping max inf

6 files changed, 67 insertions, 36 deletions

diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json
index 73d4b99b382..5b990ca8758 100644
--- a/container-search/abi-spec.json
+++ b/container-search/abi-spec.json
@@ -2927,6 +2927,7 @@
       "public com.yahoo.search.grouping.request.GroupingOperation setMax(int)",
       "public int getMax()",
       "public boolean hasMax()",
+      "public boolean hasUnlimitedMax()",
       "public com.yahoo.search.grouping.request.GroupingOperation setAccuracy(double)",
       "public double getAccuracy()",
       "public com.yahoo.search.grouping.request.GroupingOperation addOrderBy(com.yahoo.search.grouping.request.GroupingExpression)",
@@ -2951,7 +2952,9 @@
       "public static java.util.List fromStringAsList(java.lang.String)",
       "public bridge synthetic com.yahoo.search.grouping.request.GroupingNode setLabel(java.lang.String)"
     ],
-    "fields": []
+    "fields": [
+      "public static final int UNLIMITED_MAX"
+    ]
   },
   "com.yahoo.search.grouping.request.HourOfDayFunction": {
     "superClass": "com.yahoo.search.grouping.request.FunctionNode",
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
index a4934586b3f..db9585e0637 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.grouping.request;
 
+import com.yahoo.api.annotations.Beta;
 import com.yahoo.collections.LazyMap;
 import com.yahoo.collections.LazySet;
 import com.yahoo.search.grouping.request.parser.GroupingParser;
@@ -25,6 +26,8 @@ import java.util.Set;
  */
 public abstract class GroupingOperation extends GroupingNode {
 
+    @Beta public static final int UNLIMITED_MAX = Integer.MAX_VALUE;
+
     private final List<GroupingExpression> orderBy = new ArrayList<>();
     private final List<GroupingExpression> outputs = new ArrayList<>();
     private final List<GroupingOperation> children = new ArrayList<>();
@@ -269,6 +272,8 @@ public abstract class GroupingOperation extends GroupingNode {
     /** Indicates if the 'max' value has been set. */
     public boolean hasMax() { return max >= 0; }
 
+    @Beta public boolean hasUnlimitedMax() { return max == Integer.MAX_VALUE; }
+
     /**
      * Assigns an accuracy value for this. This is a number between 0 and 1 describing the accuracy of the result, which
      * again determines the speed of the grouping request. A low value will make sure the grouping operation runs fast,
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java
index 5ee6f7bc604..78452b36cd5 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java
@@ -179,16 +179,10 @@ class RequestBuilder {
                 grpLevel.setPrecision(frame.state.precision + offset);
                 frame.state.precision = null;
             }
-            int max = -1;
             if (frame.state.max != null) {
-                max = frame.state.max;
+                transform.putMax(tag, frame.state.max, "group list");
+                grpLevel.setMaxGroups(LOOKAHEAD + frame.state.max + offset);
                 frame.state.max = null;
-            } else if (defaultMaxGroups >= 0) {
-                max = defaultMaxGroups;
-            }
-            if (max >= 0) {
-                transform.putMax(tag, max, "group list");
-                grpLevel.setMaxGroups(LOOKAHEAD + max + offset);
             }
             frame.grouping.getLevels().add(grpLevel);
         }
@@ -245,12 +239,19 @@ class RequestBuilder {
         return (oldMax < 0) ? newMax : Math.min(oldMax, newMax);
     }
     private void resolveMax(BuildFrame frame) {
-        if (frame.astNode.hasMax()) {
-            int max = frame.astNode.getMax();
-            if (isTopNAllowed(frame)) {
-                frame.grouping.setTopN(computeNewTopN(frame.grouping.getTopN(), max));
-            } else {
-                frame.state.max = max;
+        if (isTopNAllowed(frame)) {
+            if (frame.astNode.hasMax() && !frame.astNode.hasUnlimitedMax()) {
+                frame.grouping.setTopN(computeNewTopN(frame.grouping.getTopN(), frame.astNode.getMax()));
+            }
+        } else {
+            if (frame.astNode.hasUnlimitedMax()) {
+                frame.state.max = null;
+            } else if (frame.astNode.hasMax()) {
+                frame.state.max = frame.astNode.getMax();
+            } else if (frame.state.groupBy != null && defaultMaxGroups != -1) {
+                frame.state.max = defaultMaxGroups;
+            } else if (frame.state.groupBy == null && defaultMaxHits != -1) {
+                frame.state.max = defaultMaxHits;
             }
         }
     }
@@ -297,17 +298,11 @@ class RequestBuilder {
                 throw new UnsupportedOperationException("Can not label expression '" + exp + "'.");
             }
             HitsAggregationResult hits = (HitsAggregationResult)result;
-            int max = -1;
             if (frame.state.max != null) {
-                max = frame.state.max;
-                frame.state.max = null;
-            } else if (defaultMaxHits >= 0) {
-                max = defaultMaxHits;
-            }
-            if (max >= 0) {
-                transform.putMax(tag, max, "hit list");
+                transform.putMax(tag, frame.state.max, "hit list");
                 int offset = transform.getOffset(tag);
-                hits.setMaxHits(LOOKAHEAD + max + offset);
+                hits.setMaxHits(LOOKAHEAD + frame.state.max + offset);
+                frame.state.max = null;
             }
             transform.putLabel(group.getTag(), tag, frame.state.label, "hit list");
         } else {
diff --git a/container-search/src/main/javacc/com/yahoo/search/grouping/request/parser/GroupingParser.jj b/container-search/src/main/javacc/com/yahoo/search/grouping/request/parser/GroupingParser.jj
index 6d4ca1e9841..8d7ad7a3ffd 100644
--- a/container-search/src/main/javacc/com/yahoo/search/grouping/request/parser/GroupingParser.jj
+++ b/container-search/src/main/javacc/com/yahoo/search/grouping/request/parser/GroupingParser.jj
@@ -245,6 +245,7 @@ GroupingOperation eachOperation(GroupingOperation parent) :
 
 void operationBody(GroupingOperation parent) :
 {
+    ConstantValue<?> maxOperand = null;
     String str;
     Number num;
     GroupingExpression exp;
@@ -255,7 +256,14 @@ void operationBody(GroupingOperation parent) :
     ( ( <ACCURACY>  lbrace() num = number()                               rbrace() { parent.setAccuracy(num.doubleValue()); } ) |
       ( <ALIAS>     lbrace() str = identifier() comma() exp = exp(parent) rbrace() { parent.putAlias(str, exp); }             ) |
       ( <HINT>      lbrace() str = identifier()                           rbrace() { parent.addHint(str); }                   ) |
-      ( <MAX>       lbrace() num = number()                               rbrace() { parent.setMax(num.intValue()); }         ) |
+      ( <MAX>       lbrace() ( maxOperand = infinitePositiveValue() | maxOperand = constantValue() ) rbrace()
+      {
+            if (maxOperand instanceof InfiniteValue) {
+                parent.setMax(GroupingOperation.UNLIMITED_MAX);
+            } else {
+                parent.setMax(((Number)maxOperand.getValue()).intValue());
+            }
+      } ) |
       ( <ORDER>     lbrace() lst = expList(parent)                        rbrace() { parent.addOrderBy(lst); }                ) |
       ( <OUTPUT>    lbrace() lst = expList(parent)                        rbrace() { parent.addOutputs(lst); }                ) |
       ( <PRECISION> lbrace() num = number()                               rbrace() { parent.setPrecision(num.intValue()); }   ) |
diff --git a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java
index c42aa905dd4..0dfd4685c0f 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java
@@ -16,6 +16,7 @@ import java.util.Arrays;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -579,6 +580,11 @@ public class GroupingParserTestCase {
         assertIllegalArgument("all(group(debugwait(artist, 3.3, lol)))",
                               "Encountered \" <IDENTIFIER> \"lol\"\" at line 1, column 34");
         assertParse("all(group(artist) each(output(stddev(simple))))");
+
+        // Test max()
+        assertTrue(assertParse("all(group(artist) max(inf))").get(0).hasUnlimitedMax());
+        assertEquals(1, assertParse("all(group(artist) max(1))").get(0).getMax());
+        assertFalse(assertParse("all(group(artist))").get(0).hasMax());
     }
 
     @Test
diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java
index 29d9f7802c1..c1b9e74757b 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java
@@ -715,17 +715,31 @@ public class RequestBuilderTestCase {
     }
 
     @Test
-    public void require_that_default_max_values_from_request_builder_restricts_respects_explicit_max() {
-        RequestBuilder builder = new RequestBuilder(0)
-                .setDefaultMaxGroups(7)
-                .setDefaultMaxHits(19)
-                .setRootOperation(GroupingOperation.fromString("all(group(foo)max(11)each(max(21)each(output(summary()))))"));
-        builder.build();
-        List<Grouping> requests = builder.getRequestList();
-        assertEquals(12, requests.get(0).getLevels().get(0).getMaxGroups());
-        HitsAggregationResult hitsAggregation =
-                (HitsAggregationResult)requests.get(0).getLevels().get(0).getGroupPrototype().getAggregationResults().get(0);
-        assertEquals(22, hitsAggregation.getMaxHits());
+    public void require_that_default_max_values_from_request_builder_respects_explicit_max() {
+        {
+            RequestBuilder builder = new RequestBuilder(0)
+                    .setDefaultMaxGroups(7)
+                    .setDefaultMaxHits(19)
+                    .setRootOperation(GroupingOperation.fromString("all(group(foo)max(11)each(max(21)each(output(summary()))))"));
+            builder.build();
+            List<Grouping> requests = builder.getRequestList();
+            assertEquals(12, requests.get(0).getLevels().get(0).getMaxGroups());
+            HitsAggregationResult hitsAggregation =
+                    (HitsAggregationResult)requests.get(0).getLevels().get(0).getGroupPrototype().getAggregationResults().get(0);
+            assertEquals(22, hitsAggregation.getMaxHits());
+        }
+        {
+            RequestBuilder builder = new RequestBuilder(0)
+                    .setDefaultMaxGroups(7)
+                    .setDefaultMaxHits(19)
+                    .setRootOperation(GroupingOperation.fromString("all(group(foo)max(inf)each(max(inf)each(output(summary()))))"));
+            builder.build();
+            List<Grouping> requests = builder.getRequestList();
+            assertEquals(-1, requests.get(0).getLevels().get(0).getMaxGroups());
+            HitsAggregationResult hitsAggregation =
+                    (HitsAggregationResult)requests.get(0).getLevels().get(0).getGroupPrototype().getAggregationResults().get(0);
+            assertEquals(-1, hitsAggregation.getMaxHits());
+        }
     }
 
     private static CompositeContinuation newComposite(EncodableContinuation... conts) {