From 4aeee2a12b6686403c3a150403f55a3d1f656442 Mon Sep 17 00:00:00 2001 From: Radu Gheorghe Date: Thu, 14 May 2026 18:44:22 +0300 Subject: [PATCH] SelectParser: add support for grouping labels --- .../com/yahoo/search/query/SelectParser.java | 39 +++++++- .../java/com/yahoo/select/SelectTestCase.java | 96 +++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-) diff --git a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java index 51b7ce5b746..979cd69ec4b 100644 --- a/container-search/src/main/java/com/yahoo/search/query/SelectParser.java +++ b/container-search/src/main/java/com/yahoo/search/query/SelectParser.java @@ -11,6 +11,7 @@ import com.yahoo.collections.LazySet; import com.yahoo.geo.DistanceParser; import com.yahoo.geo.ParsedDegree; +import com.yahoo.javacc.UnicodeUtilities; import com.yahoo.language.Language; import com.yahoo.language.detect.Detector; import com.yahoo.language.process.LinguisticsParameters; @@ -358,20 +359,39 @@ private String toGroupingRequest(Inspector groupingJson) { } private void toGroupingRequest(Inspector groupingJson, StringBuilder b) { + toGroupingRequest(groupingJson, null, b); + } + + private void toGroupingRequest(Inspector groupingJson, String parentKey, StringBuilder b) { switch (groupingJson.type()) { case ARRAY: groupingJson.traverse((ArrayTraverser) (index, item) -> { - toGroupingRequest(item, b); + toGroupingRequest(item, parentKey, b); if (index + 1 < groupingJson.entries()) b.append(","); }); break; case OBJECT: groupingJson.traverse((ObjectTraverser) (name, object) -> { + if ("label".equals(name)) { + if (!"each".equals(parentKey)) { + throw new IllegalInputException( + "'label' is only valid inside an 'each' grouping operation, found inside '" + + (parentKey == null ? "" : parentKey) + "'"); + } + return; + } b.append(name); b.append("("); - toGroupingRequest(object, b); - b.append(") "); + toGroupingRequest(object, name, b); + b.append(")"); + if ("each".equals(name) && object.type() == OBJECT) { + String label = readLabel(object); + if (label != null) { + b.append(" as(").append(UnicodeUtilities.quote(label, '"')).append(")"); + } + } + b.append(" "); }); break; case STRING: @@ -383,6 +403,19 @@ private void toGroupingRequest(Inspector groupingJson, StringBuilder b) { } } + private static String readLabel(Inspector eachBody) { + Inspector field = eachBody.field("label"); + if (!field.valid()) return null; + if (field.type() != STRING) { + throw new IllegalInputException("'label' in grouping must be a string, got " + field.type()); + } + String s = field.asString(); + if (s.isBlank()) { + throw new IllegalInputException("'label' in grouping must not be empty or whitespace"); + } + return s; + } + private Item buildFunctionCall(String key, Inspector value) { return switch (key) { case WAND -> buildWand(key, value); diff --git a/container-search/src/test/java/com/yahoo/select/SelectTestCase.java b/container-search/src/test/java/com/yahoo/select/SelectTestCase.java index 0a415643e8c..cd6916ea075 100644 --- a/container-search/src/test/java/com/yahoo/select/SelectTestCase.java +++ b/container-search/src/test/java/com/yahoo/select/SelectTestCase.java @@ -33,6 +33,7 @@ import com.yahoo.search.grouping.request.AttributeValue; import com.yahoo.search.grouping.request.CountAggregator; import com.yahoo.search.grouping.request.EachOperation; +import com.yahoo.search.grouping.request.GroupingOperation; import com.yahoo.search.grouping.request.MaxAggregator; import com.yahoo.search.grouping.request.MinAggregator; import com.yahoo.search.query.QueryTree; @@ -57,6 +58,7 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -916,6 +918,100 @@ void testMultipleOutputs() { assertGrouping(expected, parseGrouping(grouping)); } + @Test + void testGroupingWithIdentifierLabel() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : \"mylabel\" } } } ]"; + String expected = "[[]all(group(a) each(output(count())) as(mylabel))]"; + assertGrouping(expected, parseGrouping(grouping)); + } + + @Test + void testGroupingWithLabelContainingSpaces() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : \"my label\" } } } ]"; + GroupingOperation root = parseGrouping(grouping).get(0).getOperation(); + EachOperation each = (EachOperation) root.getChild(0); + assertEquals("my label", each.getLabel()); + } + + @Test + void testGroupingWithLabelContainingQuoteAndBackslash() { + // Raw JSON: "label" : "a\"b\\c" -> decoded label should be the 5-char string a"b\c + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : \"a\\\"b\\\\c\" } } } ]"; + GroupingOperation root = parseGrouping(grouping).get(0).getOperation(); + EachOperation each = (EachOperation) root.getChild(0); + assertEquals("a\"b\\c", each.getLabel()); + } + + @Test + void testGroupingWithLabelOnInnerEachOnly() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"group\" : \"b\", \"each\" : { \"output\" : \"count()\", \"label\" : \"inner\" } } } } ]"; + GroupingOperation root = parseGrouping(grouping).get(0).getOperation(); + EachOperation outer = (EachOperation) root.getChild(0); + EachOperation inner = (EachOperation) outer.getChild(0); + assertNull(outer.getLabel()); + assertEquals("inner", inner.getLabel()); + } + + @Test + void testMultipleGroupingsOnlyOneLabeled() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\" } } }," + + " { \"all\" : { \"group\" : \"b\", \"each\" : { \"output\" : \"count()\", \"label\" : \"second\" } } } ]"; + String expected = "[[]all(group(a) each(output(count()))), []all(group(b) each(output(count())) as(second))]"; + assertGrouping(expected, parseGrouping(grouping)); + } + + @Test + void testGroupingLabelInsideAllIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"label\" : \"x\", \"each\" : { \"output\" : \"count()\" } } } ]"; + var e = assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + assertTrue(e.getMessage().contains("'all'"), "Message should name 'all' parent: " + e.getMessage()); + } + + @Test + void testGroupingLabelInsideOutputObjectIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : { \"label\" : \"x\" } } } } ]"; + var e = assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + assertTrue(e.getMessage().contains("'output'"), "Message should name 'output' parent: " + e.getMessage()); + } + + @Test + void testGroupingLabelInsideOutputArrayElementIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : [ \"count()\", { \"label\" : \"x\" } ] } } } ]"; + var e = assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + assertTrue(e.getMessage().contains("'output'"), "Message should name 'output' parent: " + e.getMessage()); + } + + @Test + void testGroupingLabelAtTopLevelIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\" } }, \"label\" : \"x\" } ]"; + var e = assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + assertTrue(e.getMessage().contains(""), "Message should name parent: " + e.getMessage()); + } + + @Test + void testGroupingLabelNumberIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : 5 } } } ]"; + assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + } + + @Test + void testGroupingLabelNullIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : null } } } ]"; + assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + } + + @Test + void testGroupingLabelEmptyIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : \"\" } } } ]"; + assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + } + + @Test + void testGroupingLabelWhitespaceOnlyIsRejected() { + String grouping = "[ { \"all\" : { \"group\" : \"a\", \"each\" : { \"output\" : \"count()\", \"label\" : \" \" } } } ]"; + assertThrows(IllegalInputException.class, () -> parseGrouping(grouping)); + } + //------------------------------------------------------------------- Other tests @Test