Skip to content

Commit

Permalink
Merge pull request #33028 from vespa-engine/toregge/use-more-readable…
Browse files Browse the repository at this point in the history 
…-type-names-in-grouping-validator

Use more readable type names in grouping validator.
  • Loading branch information
@geirst
geirst authored Dec 12, 2024
2 parents 68dde35 + c728a2e commit 4bf1aff
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 22 deletions.
44 changes: 39 additions & 5 deletions container-search/src/main/java/com/yahoo/search/grouping/GroupingValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,40 @@ public Result search(Query query, Execution execution) {
return execution.search(query);
}

static private String datatypeAsString(AttributesConfig.Attribute attribute) {
var datatype = attribute.datatype();
if (datatype == AttributesConfig.Attribute.Datatype.TENSOR && !attribute.tensortype().isEmpty()) {
return attribute.tensortype();
}
return switch (datatype) {
case STRING -> "string";
case BOOL -> "bool";
case UINT2 -> "uint2";
case UINT4 -> "uint4";
case INT8 -> "byte";
case INT16 -> "short";
case INT32 -> "int";
case INT64 -> "long";
case FLOAT16 -> "float16";
case FLOAT -> "float";
case DOUBLE -> "double";
case PREDICATE -> "predicate";
case TENSOR -> "tensor";
case REFERENCE -> "reference";
case RAW -> "raw";
case NONE -> "none";
};
}

static private String typeAsString(AttributesConfig.Attribute attribute) {
var collectiontype = attribute.collectiontype();
return switch(collectiontype) {
case SINGLE -> datatypeAsString(attribute);
case ARRAY -> "array<" + datatypeAsString(attribute) + ">";
case WEIGHTEDSET -> "weightedset<" + datatypeAsString(attribute) + ">";
};
}

static private boolean isPrimitiveAttribute(AttributesConfig.Attribute attribute) {
var datatype = attribute.datatype();
return datatype == AttributesConfig.Attribute.Datatype.INT8 ||
Expand Down Expand Up @@ -92,8 +126,7 @@ private void verifyHasAttribute(String attributeName, boolean isMapLookup) {
return;
}
throw new IllegalInputException("Grouping request references attribute '" +
attributeName + "' with unsupported data type '" + attribute.datatype() +
"' collection type '" + attribute.collectiontype() + "'" +
attributeName + "' with unsupported type '" + typeAsString(attribute) + "'" +
(isMapLookup ? " for map lookup" : ""));
}

Expand All @@ -103,13 +136,14 @@ private void verifyCompatibleAttributeTypes(String keyAttributeName,
AttributesConfig.Attribute keySourceAttribute = attributes.get(keySourceAttributeName);
if (!keySourceAttribute.datatype().equals(keyAttribute.datatype())) {
throw new IllegalInputException("Grouping request references key source attribute '" +
keySourceAttributeName + "' with data type '" + keySourceAttribute.datatype() +
"' that is different than data type '" + keyAttribute.datatype() + "' of key attribute '" +
keySourceAttributeName + "' with data type '" + datatypeAsString(keySourceAttribute) +
"' that is different than data type '" + datatypeAsString(keyAttribute) + "' of key attribute '" +
keyAttributeName + "'");
}
if (!keySourceAttribute.collectiontype().equals(AttributesConfig.Attribute.Collectiontype.Enum.SINGLE)) {
throw new IllegalInputException("Grouping request references key source attribute '" +
keySourceAttributeName + "' which is not of single value type");
keySourceAttributeName + "' with type '" + typeAsString(keySourceAttribute) + "' " +
"which is not of single value type");
}
}

Expand Down
34 changes: 17 additions & 17 deletions container-search/src/test/java/com/yahoo/search/grouping/GroupingValidatorTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ void key_source_attribute_with_mismatching_data_type_throws() {
fail("Expected exception");
}
catch (IllegalArgumentException e) {
assertEquals("Grouping request references key source attribute 'key_source' with data type 'INT32' " +
"that is different than data type 'STRING' of key attribute 'map.key'",
assertEquals("Grouping request references key source attribute 'key_source' with data type 'int' " +
"that is different than data type 'string' of key attribute 'map.key'",
e.getMessage());
}
}
Expand All @@ -140,56 +140,56 @@ void key_source_attribute_with_multi_value_collection_type_throws() {
fail("Expected exception");
}
catch (IllegalArgumentException e) {
assertEquals("Grouping request references key source attribute 'key_source' which is not of single value type",
assertEquals("Grouping request references key source attribute 'key_source' with type 'array<string>' which is not of single value type",
e.getMessage());
}
}

private static void unsupported_attribute_type_throws(String name, AttributesConfig.Attribute.Datatype.Enum datatype) {
private static void unsupported_attribute_type_throws(String name,
AttributesConfig.Attribute.Datatype.Enum datatype,
String typeName) {
try {
var builder = new AttributesConfig.Builder();
builder.attribute(new AttributesConfig.Attribute.Builder().name(name).datatype(datatype));
validateGrouping(new AttributesConfig(builder),
"all(group(" + name + ") each(output(count())))");
validate_attribute_type(name, datatype);
fail("Expected exception");
} catch (IllegalArgumentException e) {
assertEquals("Grouping request references attribute '" + name + "' " +
"with unsupported data type '" + datatype.toString() + "' collection type 'SINGLE'",
"with unsupported type '" + typeName + "'",
e.getMessage());
}
}

private static void supported_attribute_type_works(String name, AttributesConfig.Attribute.Datatype.Enum datatype) {
private static void validate_attribute_type(String name, AttributesConfig.Attribute.Datatype.Enum datatype) {
var builder = new AttributesConfig.Builder();
builder.attribute(new AttributesConfig.Attribute.Builder().name(name).datatype(datatype));
builder.attribute(new AttributesConfig.Attribute.Builder().name(name).datatype(datatype)
.tensortype(datatype == AttributesConfig.Attribute.Datatype.TENSOR ? "tensor(x[2])" : ""));
validateGrouping(new AttributesConfig(builder),
"all(group(" + name + ") each(output(count())))");

}

@Test
void tensor_attribute_throws() {
unsupported_attribute_type_throws("tensor", AttributesConfig.Attribute.Datatype.TENSOR);
unsupported_attribute_type_throws("tensor", AttributesConfig.Attribute.Datatype.TENSOR, "tensor(x[2])");
}

@Test
void predicate_attribute_throws() {
unsupported_attribute_type_throws("predicate", AttributesConfig.Attribute.Datatype.PREDICATE);
unsupported_attribute_type_throws("predicate", AttributesConfig.Attribute.Datatype.PREDICATE, "predicate");
}

@Test
void reference_attribute_throws() {
unsupported_attribute_type_throws("reference", AttributesConfig.Attribute.Datatype.REFERENCE);
unsupported_attribute_type_throws("reference", AttributesConfig.Attribute.Datatype.REFERENCE, "reference");
}

@Test
void raw_attribute_is_ok() {
supported_attribute_type_works("raw", AttributesConfig.Attribute.Datatype.RAW);
validate_attribute_type("raw", AttributesConfig.Attribute.Datatype.RAW);
}

@Test
void bool_attribute_is_ok() {
supported_attribute_type_works("mybool", AttributesConfig.Attribute.Datatype.BOOL);
validate_attribute_type("mybool", AttributesConfig.Attribute.Datatype.BOOL);
}

@Test
Expand All @@ -207,7 +207,7 @@ void raw_attribute_cannot_be_used_for_lookup() {
fail("Expected exception");
} catch (IllegalArgumentException e) {
assertEquals("Grouping request references attribute 'map.key' " +
"with unsupported data type 'RAW' collection type 'SINGLE' for map lookup",
"with unsupported type 'raw' for map lookup",
e.getMessage());
}
}
Expand Down

0 comments on commit 4bf1aff

Please sign in to comment.