diff options
3 files changed, 122 insertions, 48 deletions
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java b/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java index 790b5c4e601..06b030dbc78 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java @@ -5,6 +5,7 @@ import com.google.inject.Inject; import com.yahoo.component.chain.dependencies.After; import com.yahoo.component.chain.dependencies.Before; import com.yahoo.component.chain.dependencies.Provides; +import com.yahoo.search.grouping.request.AttributeMapLookupValue; import com.yahoo.vespa.config.search.AttributesConfig; import com.yahoo.container.QrSearchersConfig; import com.yahoo.processing.request.CompoundName; @@ -18,8 +19,7 @@ import com.yahoo.search.grouping.request.GroupingExpression; import com.yahoo.search.searchchain.Execution; import com.yahoo.search.searchchain.PhaseNames; -import java.util.HashSet; -import java.util.Set; +import java.util.HashMap; import static com.yahoo.search.grouping.GroupingQueryParser.SELECT_PARAMETER_PARSING; @@ -37,7 +37,7 @@ public class GroupingValidator extends Searcher { public static final String GROUPING_VALIDATED = "GroupingValidated"; public static final CompoundName PARAM_ENABLED = new CompoundName("validate_" + GroupingQueryParser.PARAM_REQUEST); - private final Set<String> attributeNames = new HashSet<>(); + private final HashMap<String, AttributesConfig.Attribute> attributes = new HashMap<>(); private final String clusterName; private final boolean enabled; @@ -55,7 +55,7 @@ public class GroupingValidator extends Searcher { enabled = (indexingMode != QrSearchersConfig.Searchcluster.Indexingmode.STREAMING); clusterName = enabled ? qrsConfig.searchcluster(clusterId).name() : null; for (AttributesConfig.Attribute attr : attributesConfig.attribute()) { - attributeNames.add(attr.name()); + attributes.put(attr.name(), attr); } } @@ -69,29 +69,42 @@ public class GroupingValidator extends Searcher { return execution.search(query); } + private void verifyHasAttribute(String attributeName) { + if (!attributes.containsKey(attributeName)) { + throw new UnavailableAttributeException(clusterName, attributeName); + } + } + + private void verifyCompatibleAttributeTypes(String keyAttributeName, + String keySourceAttributeName) { + AttributesConfig.Attribute keyAttribute = attributes.get(keyAttributeName); + AttributesConfig.Attribute keySourceAttribute = attributes.get(keySourceAttributeName); + if (!keySourceAttribute.datatype().equals(keyAttribute.datatype())) { + throw new IllegalArgumentException("Grouping request references key source attribute '" + + keySourceAttributeName + "' with data type '" + keySourceAttribute.datatype() + + "' that is different than data type '" + keyAttribute.datatype() + "' of key attribute '" + + keyAttributeName + "'"); + } + if (!keySourceAttribute.collectiontype().equals(AttributesConfig.Attribute.Collectiontype.Enum.SINGLE)) { + throw new IllegalArgumentException("Grouping request references key source attribute '" + + keySourceAttributeName + "' which is not of single value type"); + } + } + private class MyVisitor implements ExpressionVisitor { @Override public void visitExpression(GroupingExpression exp) { - if (exp instanceof AttributeValue) { - String name = ((AttributeValue)exp).getAttributeName(); - int leftBracePos = name.indexOf('{'); - if (leftBracePos == -1) { - if (!attributeNames.contains(name)) { - throw new UnavailableAttributeException(clusterName, name); - } - } else { - String baseName = name.substring(0, leftBracePos); - String keyName = baseName + ".key"; - if (!attributeNames.contains(keyName)) { - throw new UnavailableAttributeException(clusterName, keyName); - } - int rightBracePos = name.lastIndexOf('}'); - String valueName = baseName + ".value" + name.substring(rightBracePos + 1); - if (!attributeNames.contains(valueName)) { - throw new UnavailableAttributeException(clusterName, valueName); - } + if (exp instanceof AttributeMapLookupValue) { + AttributeMapLookupValue mapLookup = (AttributeMapLookupValue) exp; + verifyHasAttribute(mapLookup.getKeyAttribute()); + verifyHasAttribute(mapLookup.getValueAttribute()); + if (mapLookup.hasKeySourceAttribute()) { + verifyHasAttribute(mapLookup.getKeySourceAttribute()); + verifyCompatibleAttributeTypes(mapLookup.getKeyAttribute(), mapLookup.getKeySourceAttribute()); } + } else if (exp instanceof AttributeValue) { + verifyHasAttribute(((AttributeValue) exp).getAttributeName()); } } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/request/AttributeMapLookupValue.java b/container-search/src/main/java/com/yahoo/search/grouping/request/AttributeMapLookupValue.java index fb9702cc1cc..82c4b6763d8 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/request/AttributeMapLookupValue.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/request/AttributeMapLookupValue.java @@ -51,6 +51,10 @@ public class AttributeMapLookupValue extends AttributeValue { return key; } + public boolean hasKeySourceAttribute() { + return !keySourceAttribute.isEmpty(); + } + public String getKeySourceAttribute() { return keySourceAttribute; } diff --git a/container-search/src/test/java/com/yahoo/search/grouping/GroupingValidatorTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/GroupingValidatorTestCase.java index 055ea5929db..9723f96af27 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/GroupingValidatorTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/GroupingValidatorTestCase.java @@ -24,15 +24,15 @@ public class GroupingValidatorTestCase { @Test public void requireThatAvailableAttributesDoNotThrow() { - validateGrouping("myCluster", Arrays.asList("foo", "bar"), + validateGrouping(Arrays.asList("foo", "bar"), "all(group(foo) each(output(max(bar))))");; } @Test public void requireThatUnavailableAttributesThrow() { thrown.expect(UnavailableAttributeException.class); - thrown.expectMessage(createMessage("myCluster", "bar")); - validateGrouping("myCluster", Arrays.asList("foo"), + thrown.expectMessage(createMessage("bar")); + validateGrouping(Arrays.asList("foo"), "all(group(foo) each(output(max(bar))))"); } @@ -40,55 +40,100 @@ public class GroupingValidatorTestCase { public void requireThatEnableFlagPreventsThrow() { Query query = createQuery("all(group(foo) each(output(max(bar))))"); query.properties().set(GroupingValidator.PARAM_ENABLED, "false"); - validateGrouping("myCluster", Arrays.asList("foo"), query); + validateGrouping(Arrays.asList("foo"), query); } @Test public void available_primitive_map_attribute_does_not_throw() { - validateGrouping("myCluster", Arrays.asList("map.key", "map.value"), + validateGrouping(Arrays.asList("map.key", "map.value"), "all(group(map{\"foo\"}) each(output(count())))"); } @Test public void unavailable_primitive_map_key_attribute_throws() { thrown.expect(UnavailableAttributeException.class); - thrown.expectMessage(createMessage("myCluster", "map.key")); - validateGrouping("myCluster", Arrays.asList("null"), + thrown.expectMessage(createMessage("map.key")); + validateGrouping(Arrays.asList("null"), "all(group(map{\"foo\"}) each(output(count())))"); } @Test public void unavailable_primitive_map_value_attribute_throws() { thrown.expect(UnavailableAttributeException.class); - thrown.expectMessage(createMessage("myCluster", "map.value")); - validateGrouping("myCluster", Arrays.asList("map.key"), + thrown.expectMessage(createMessage("map.value")); + validateGrouping(Arrays.asList("map.key"), "all(group(map{\"foo\"}) each(output(count())))"); } @Test public void available_struct_map_attribute_does_not_throw() { - validateGrouping("myCluster", Arrays.asList("map.key", "map.value.name"), + validateGrouping(Arrays.asList("map.key", "map.value.name"), "all(group(map{\"foo\"}.name) each(output(count())))"); } @Test public void unavailable_struct_map_key_attribute_throws() { thrown.expect(UnavailableAttributeException.class); - thrown.expectMessage(createMessage("myCluster", "map.key")); - validateGrouping("myCluster", Arrays.asList("null"), + thrown.expectMessage(createMessage("map.key")); + validateGrouping(Arrays.asList("null"), "all(group(map{\"foo\"}.name) each(output(count())))"); } @Test public void unavailable_struct_map_value_attribute_throws() { thrown.expect(UnavailableAttributeException.class); - thrown.expectMessage(createMessage("myCluster", "map.value.name")); - validateGrouping("myCluster", Arrays.asList("map.key"), + thrown.expectMessage(createMessage("map.value.name")); + validateGrouping(Arrays.asList("map.key"), "all(group(map{\"foo\"}.name) each(output(count())))"); } - private static String createMessage(String clusterName, String attributeName) { - return "Grouping request references attribute '" + attributeName + "' which is not available in cluster '" + clusterName + "'."; + @Test + public void available_key_source_attribute_does_not_throw() { + validateGrouping(Arrays.asList("map.key", "map.value", "key_source"), + "all(group(map{attribute(key_source)}) each(output(count())))"); + } + + @Test + public void unavailable_key_source_attribute_throws() { + thrown.expect(UnavailableAttributeException.class); + thrown.expectMessage(createMessage("key_source")); + validateGrouping(Arrays.asList("map.key", "map.value"), + "all(group(map{attribute(key_source)}) each(output(count())))"); + } + + @Test + public void key_source_attribute_with_mismatching_data_type_throws() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Grouping request references key source attribute 'key_source' with data type 'INT32' " + + "that is different than data type 'STRING' of key attribute 'map.key'"); + + validateGrouping(setupMismatchingKeySourceAttribute(false), + "all(group(map{attribute(key_source)}) each(output(count())))"); + } + + @Test + public void key_source_attribute_with_multi_value_collection_type_throws() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Grouping request references key source attribute 'key_source' which is not of single value type"); + + validateGrouping(setupMismatchingKeySourceAttribute(true), + "all(group(map{attribute(key_source)}) each(output(count())))"); + } + + private static AttributesConfig setupMismatchingKeySourceAttribute(boolean matchingDataType) { + AttributesConfig.Builder builder = new AttributesConfig.Builder(); + builder.attribute(new AttributesConfig.Attribute.Builder().name("map.key") + .datatype(AttributesConfig.Attribute.Datatype.Enum.STRING)); + builder.attribute(new AttributesConfig.Attribute.Builder().name("map.value")); + builder.attribute(new AttributesConfig.Attribute.Builder().name("key_source") + .datatype(matchingDataType ? AttributesConfig.Attribute.Datatype.Enum.STRING : + AttributesConfig.Attribute.Datatype.Enum.INT32) + .collectiontype(AttributesConfig.Attribute.Collectiontype.Enum.ARRAY)); + return new AttributesConfig(builder); + } + + private static String createMessage(String attributeName) { + return "Grouping request references attribute '" + attributeName + "' which is not available in cluster 'myCluster'."; } private static Query createQuery(String groupingExpression) { @@ -98,11 +143,28 @@ public class GroupingValidatorTestCase { return query; } - private static void validateGrouping(String clusterName, Collection<String> attributeNames, String groupingExpression) { - validateGrouping(clusterName, attributeNames, createQuery(groupingExpression)); + private static AttributesConfig createAttributesConfig(Collection<String> attributeNames) { + AttributesConfig.Builder builder = new AttributesConfig.Builder(); + for (String attributeName : attributeNames) { + builder.attribute(new AttributesConfig.Attribute.Builder() + .name(attributeName)); + } + return new AttributesConfig(builder); + } + + private static void validateGrouping(Collection<String> attributeNames, String groupingExpression) { + validateGrouping("myCluster", createAttributesConfig(attributeNames), createQuery(groupingExpression)); } - private static void validateGrouping(String clusterName, Collection<String> attributeNames, Query query) { + private static void validateGrouping(AttributesConfig attributesCfg, String groupingExpression) { + validateGrouping("myCluster", attributesCfg, createQuery(groupingExpression)); + } + + private static void validateGrouping(Collection<String> attributeNames, Query query) { + validateGrouping("myCluster", createAttributesConfig(attributeNames), query); + } + + private static void validateGrouping(String clusterName, AttributesConfig attributesConfig, Query query) { QrSearchersConfig.Builder qrsConfig = new QrSearchersConfig.Builder().searchcluster( new QrSearchersConfig.Searchcluster.Builder() .indexingmode(QrSearchersConfig.Searchcluster.Indexingmode.Enum.REALTIME) @@ -110,15 +172,10 @@ public class GroupingValidatorTestCase { ClusterConfig.Builder clusterConfig = new ClusterConfig.Builder(). clusterId(0). clusterName("test"); - AttributesConfig.Builder attributesConfig = new AttributesConfig.Builder(); - for (String attributeName : attributeNames) { - attributesConfig.attribute(new AttributesConfig.Attribute.Builder() - .name(attributeName)); - } new Execution( new GroupingValidator(new QrSearchersConfig(qrsConfig), - new ClusterConfig(clusterConfig), - new AttributesConfig(attributesConfig)), + new ClusterConfig(clusterConfig), + attributesConfig), Execution.Context.createContextStub()).search(query); } } |