From da82f0eb66032d2e2de918bc60fe0dfb2a45d610 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 23 Mar 2022 14:10:50 +0100 Subject: Allow 'inf' as value to max() --- container-search/abi-spec.json | 5 ++- .../search/grouping/request/GroupingOperation.java | 5 +++ .../search/grouping/vespa/RequestBuilder.java | 15 ++++----- .../grouping/request/parser/GroupingParser.jj | 10 +++++- .../request/parser/GroupingParserTestCase.java | 6 ++++ .../grouping/vespa/RequestBuilderTestCase.java | 36 +++++++++++++++------- 6 files changed, 57 insertions(+), 20 deletions(-) diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 8ac5aaa127d..4d559fcb985 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 orderBy = new ArrayList<>(); private final List outputs = new ArrayList<>(); private final List 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..a77e83e0078 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,14 +179,14 @@ class RequestBuilder { grpLevel.setPrecision(frame.state.precision + offset); frame.state.precision = null; } - int max = -1; + int max = GroupingOperation.UNSPECIFIED_MAX; if (frame.state.max != null) { max = frame.state.max; frame.state.max = null; } else if (defaultMaxGroups >= 0) { max = defaultMaxGroups; } - if (max >= 0) { + if (max >= 0 && max != GroupingOperation.UNLIMITED_MAX) { transform.putMax(tag, max, "group list"); grpLevel.setMaxGroups(LOOKAHEAD + max + offset); } @@ -246,11 +246,12 @@ class RequestBuilder { } 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)); + if (!frame.astNode.hasUnlimitedMax()) { + frame.grouping.setTopN(computeNewTopN(frame.grouping.getTopN(), frame.astNode.getMax())); + } } else { - frame.state.max = max; + frame.state.max = frame.astNode.getMax(); } } } @@ -297,14 +298,14 @@ class RequestBuilder { throw new UnsupportedOperationException("Can not label expression '" + exp + "'."); } HitsAggregationResult hits = (HitsAggregationResult)result; - int max = -1; + int max = GroupingOperation.UNSPECIFIED_MAX; if (frame.state.max != null) { max = frame.state.max; frame.state.max = null; } else if (defaultMaxHits >= 0) { max = defaultMaxHits; } - if (max >= 0) { + if (max >= 0 && max != GroupingOperation.UNLIMITED_MAX) { transform.putMax(tag, max, "hit list"); int offset = transform.getOffset(tag); hits.setMaxHits(LOOKAHEAD + max + offset); 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) : ( ( lbrace() num = number() rbrace() { parent.setAccuracy(num.doubleValue()); } ) | ( lbrace() str = identifier() comma() exp = exp(parent) rbrace() { parent.putAlias(str, exp); } ) | ( lbrace() str = identifier() rbrace() { parent.addHint(str); } ) | - ( lbrace() num = number() rbrace() { parent.setMax(num.intValue()); } ) | + ( lbrace() ( maxOperand = infinitePositiveValue() | maxOperand = constantValue() ) rbrace() + { + if (maxOperand instanceof InfiniteValue) { + parent.setMax(GroupingOperation.UNLIMITED_MAX); + } else { + parent.setMax(((Number)maxOperand.getValue()).intValue()); + } + } ) | ( lbrace() lst = expList(parent) rbrace() { parent.addOrderBy(lst); } ) | ( lbrace() lst = expList(parent) rbrace() { parent.addOutputs(lst); } ) | ( 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 \" \"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 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 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 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) { -- cgit v1.2.3 From 9b505f09092c7df8900da85b9f1d08149466c96d Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 23 Mar 2022 14:54:41 +0100 Subject: Move all max resolving to resolveMax --- .../search/grouping/vespa/RequestBuilder.java | 40 +++++++++------------- 1 file changed, 17 insertions(+), 23 deletions(-) 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 a77e83e0078..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 = GroupingOperation.UNSPECIFIED_MAX; 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 && max != GroupingOperation.UNLIMITED_MAX) { - transform.putMax(tag, max, "group list"); - grpLevel.setMaxGroups(LOOKAHEAD + max + offset); } frame.grouping.getLevels().add(grpLevel); } @@ -245,13 +239,19 @@ class RequestBuilder { return (oldMax < 0) ? newMax : Math.min(oldMax, newMax); } private void resolveMax(BuildFrame frame) { - if (frame.astNode.hasMax()) { - if (isTopNAllowed(frame)) { - if (!frame.astNode.hasUnlimitedMax()) { - frame.grouping.setTopN(computeNewTopN(frame.grouping.getTopN(), frame.astNode.getMax())); - } - } else { + 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; } } } @@ -298,17 +298,11 @@ class RequestBuilder { throw new UnsupportedOperationException("Can not label expression '" + exp + "'."); } HitsAggregationResult hits = (HitsAggregationResult)result; - int max = GroupingOperation.UNSPECIFIED_MAX; if (frame.state.max != null) { - max = frame.state.max; - frame.state.max = null; - } else if (defaultMaxHits >= 0) { - max = defaultMaxHits; - } - if (max >= 0 && max != GroupingOperation.UNLIMITED_MAX) { - 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 { -- cgit v1.2.3