Skip to content

Commit

Permalink
Merge pull request #21787 from vespa-engine/bjorncs/grouping-max-inf
Browse files Browse the repository at this point in the history 
Bjorncs/grouping max inf
  • Loading branch information
@baldersheim
baldersheim authored Mar 23, 2022
2 parents b07fc57 + 9b505f0 commit 73b080b
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 36 deletions.
5 changes: 4 additions & 1 deletion container-search/abi-spec.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand All @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions container-search/src/main/java/com/yahoo/search/grouping/request/GroupingOperation.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -25,6 +26,8 @@
*/
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<>();
Expand Down Expand Up @@ -269,6 +272,8 @@ public int getMax() {
/** 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,
Expand Down
41 changes: 18 additions & 23 deletions container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,10 @@ private void resolveEach(BuildFrame frame) {
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);
}
Expand Down Expand Up @@ -245,12 +239,19 @@ private long computeNewTopN(long oldMax, long newMax) {
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;
}
}
}
Expand Down Expand Up @@ -297,17 +298,11 @@ private AggregationResult toAggregationResult(GroupingExpression exp, Group grou
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 {
Expand Down
10 changes: 9 additions & 1 deletion container-search/src/main/javacc/com/yahoo/search/grouping/request/parser/GroupingParser.jj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ GroupingOperation eachOperation(GroupingOperation parent) :

void operationBody(GroupingOperation parent) :
{
ConstantValue<?> maxOperand = null;
String str;
Number num;
GroupingExpression exp;
Expand All @@ -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()); } ) |
Expand Down
6 changes: 6 additions & 0 deletions ...search/src/test/java/com/yahoo/search/grouping/request/parser/GroupingParserTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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;
Expand Down Expand Up @@ -579,6 +580,11 @@ public void testMisc() {
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
Expand Down
36 changes: 25 additions & 11 deletions container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,17 +715,31 @@ public void require_that_default_max_values_from_request_builder_restricts_max_g
}

@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) {
Expand Down

0 comments on commit 73b080b

Please sign in to comment.