From c728a2e50ca8b950880f2425f3f53d0b669653fe Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Thu, 12 Dec 2024 11:02:36 +0100 Subject: [PATCH] Use more readable type names in grouping validator. --- .../search/grouping/GroupingValidator.java | 44 ++++++++++++++++--- .../grouping/GroupingValidatorTestCase.java | 34 +++++++------- 2 files changed, 56 insertions(+), 22 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 27ff50e2821..70942372d2b 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 @@ -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 || @@ -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" : "")); } @@ -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"); } } 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 188db8332b0..de3254d85ad 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 @@ -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()); } } @@ -140,28 +140,28 @@ 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' 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())))"); @@ -169,27 +169,27 @@ private static void supported_attribute_type_works(String name, AttributesConfig @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 @@ -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()); } }