diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2018-02-03 09:04:46 +0100 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2018-02-03 09:17:02 +0100 |
commit | 56b68f4b23479ffe4a22ecaeec5d8e7b779ce485 (patch) | |
tree | 0631fdb246faaa314a38437dd2299ee77f8c835b /container-search | |
parent | 4026dc60fca122daff0bad8d50bd52045b6d8aee (diff) |
Propagate max() within nested all to propagate properly.
Earlier on only the one in the outer all was used.
Test that it is actually propagates properly down to all leaves.
Diffstat (limited to 'container-search')
-rw-r--r-- | container-search/src/main/java/com/yahoo/search/grouping/vespa/RequestBuilder.java | 13 | ||||
-rw-r--r-- | container-search/src/test/java/com/yahoo/search/grouping/vespa/RequestBuilderTestCase.java | 51 |
2 files changed, 61 insertions, 3 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 c6772f449b5..6a35e599eba 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 @@ -3,6 +3,7 @@ package com.yahoo.search.grouping.vespa; import com.yahoo.search.grouping.Continuation; import com.yahoo.search.grouping.GroupingRequest; +import com.yahoo.search.grouping.request.AllOperation; import com.yahoo.search.grouping.request.EachOperation; import com.yahoo.search.grouping.request.GroupingExpression; import com.yahoo.search.grouping.request.GroupingOperation; @@ -228,12 +229,14 @@ class RequestBuilder { } } + 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 (isRootOperation(frame)) { - frame.grouping.setTopN(max); + if (isTopNAllowed(frame)) { + frame.grouping.setTopN(computeNewTopN(frame.grouping.getTopN(), max)); } else { frame.state.max = max; } @@ -325,6 +328,10 @@ class RequestBuilder { return frame.astNode == root && frame.state.groupBy == null; } + private boolean isTopNAllowed(BuildFrame frame) { + return (frame.astNode instanceof AllOperation) && (frame.state.groupBy == null); + } + private GroupingLevel getLeafGroupingLevel(BuildFrame frame) { if (frame.grouping.getLevels().isEmpty()) { return null; 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 cf58dd3929b..33e8dbbbef1 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 @@ -40,6 +40,57 @@ public class RequestBuilderTestCase { assertEquals(new AttributeNode("foo"), aggr.getExpression()); } + private List<Grouping> getRequestList(String selection) { + RequestBuilder builder = new RequestBuilder(0); + builder.setRootOperation(GroupingOperation.fromString(selection)); + builder.build(); + return builder.getRequestList(); + } + + @Test + public void requireThatTopNIsHonoured() { + List<Grouping> gl = getRequestList("all(max(3) all(group(product_id) max(5) each(output(sum(price)))))"); + assertEquals(1, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + } + + @Test + public void requireThatTopNIsHonouredWhenNested() { + List<Grouping> gl = getRequestList("all( all(max(3) all(group(product_id) max(5) each(output(sum(price))))))"); + assertEquals(1, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + } + + @Test + public void requireThatTopNIsInherited() { + List<Grouping> gl = getRequestList("all(max(7) all( all(group(product_id) max(5) each(output(sum(price))))))"); + assertEquals(1, gl.size()); + assertEquals(7, gl.get(0).getTopN()); + } + + @Test + public void requireThatTopNIsMinimum() { + List<Grouping> gl = getRequestList("all(max(7) all(max(3) all(group(product_id) max(5) each(output(sum(price))))))"); + assertEquals(1, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + gl = getRequestList("all(max(3) all(max(7) all(group(product_id) max(5) each(output(sum(price))))))"); + assertEquals(1, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + } + + @Test + public void requireThatTopNIsIndividual() { + List<Grouping> gl = getRequestList("all( all(max(3) all(group(product_id) max(5) each(output(sum(price))))) all(group(filter_cluster3) order(count()) each(output(count()))))"); + assertEquals(2, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + assertEquals(-1, gl.get(1).getTopN()); + + gl = getRequestList("all( max(7) all(max(3) all(group(product_id) max(5) each(output(sum(price))))) all(group(filter_cluster3) order(count()) each(output(count()))))"); + assertEquals(2, gl.size()); + assertEquals(3, gl.get(0).getTopN()); + assertEquals(7, gl.get(1).getTopN()); + } + @Test public void requireThatAllExpressionNodesAreSupported() { assertLayout("all(group(add(a,b)) each(output(count())))", "[[{ Add, result = [Count] }]]"); |