Merge pull request #32674 from vespa-engine/bratseth/more-validate-sorting

Bratseth/more validate sorting

Author: hmusum

config-model/src/main/java/com/yahoo/schema/document/Attribute.java

@@ -109,7 +109,7 @@ public enum Type {
         private final String exportAttributeTypeName;
 
         Type(String name, String exportAttributeTypeName) {
-            this.myName=name;
+            this.myName = name;
             this.exportAttributeTypeName = exportAttributeTypeName;
         }
 

container-search/src/main/java/com/yahoo/prelude/searcher/ValidateSortingSearcher.java

@@ -29,27 +29,32 @@
 @After(ACCENT_REMOVAL)
 public class ValidateSortingSearcher extends Searcher {
 
+    // TODO: Use SchemaInfo instead and validate with streaming as well
+
+    private final boolean isStreaming;
+
     private Map<String, AttributesConfig.Attribute> attributeNames = null;
     private String clusterName = "";
-    private final boolean enabled;
 
     public ValidateSortingSearcher(ClusterConfig clusterConfig, AttributesConfig attributesConfig) {
         initAttributeNames(attributesConfig);
         setClusterName(clusterConfig.clusterName());
-        enabled = clusterConfig.indexMode() != ClusterConfig.IndexMode.Enum.STREAMING;
+        isStreaming = clusterConfig.indexMode() == ClusterConfig.IndexMode.Enum.STREAMING;
     }
 
-    public String getClusterName() {
-        return clusterName;
+    @Override
+    public Result search(Query query, Execution execution) {
+        ErrorMessage error = validate(query);
+        if (! isStreaming && error != null)
+            return new Result(query, error);
+        return execution.search(query);
     }
 
-    public void setClusterName(String clusterName) {
-        this.clusterName = clusterName;
-    }
+    public String getClusterName() { return clusterName; }
 
-    private Map<String, AttributesConfig.Attribute> getAttributeNames() {
-        return attributeNames;
-    }
+    public void setClusterName(String clusterName) { this.clusterName = clusterName; }
+
+    private Map<String, AttributesConfig.Attribute> getAttributeNames() { return attributeNames; }
 
     public void setAttributeNames(Map<String, AttributesConfig.Attribute> attributeNames) {
         this.attributeNames = attributeNames;
@@ -64,32 +69,6 @@ public void initAttributeNames(AttributesConfig config) {
         setAttributeNames(attributes);
     }
 
-    @Override
-    public Result search(Query query, Execution execution) {
-        ErrorMessage e = validate(query);
-        if (enabled && e != null) {
-            Result r = new Result(query);
-            r.hits().addError(e);
-            return r;
-        }
-        return execution.search(query);
-    }
-
-    private static Sorting.UcaSorter.Strength config2Strength(AttributesConfig.Attribute.Sortstrength.Enum s) {
-        if (s == AttributesConfig.Attribute.Sortstrength.PRIMARY) {
-            return Sorting.UcaSorter.Strength.PRIMARY;
-        } else if (s == AttributesConfig.Attribute.Sortstrength.SECONDARY) {
-            return Sorting.UcaSorter.Strength.SECONDARY;
-        } else if (s == AttributesConfig.Attribute.Sortstrength.TERTIARY) {
-            return Sorting.UcaSorter.Strength.TERTIARY;
-        } else if (s == AttributesConfig.Attribute.Sortstrength.QUATERNARY) {
-            return Sorting.UcaSorter.Strength.QUATERNARY;
-        } else if (s == AttributesConfig.Attribute.Sortstrength.IDENTICAL) {
-            return Sorting.UcaSorter.Strength.IDENTICAL;
-        }
-        return Sorting.UcaSorter.Strength.PRIMARY;
-    }
-
     private ErrorMessage validate(Query query) {
         Sorting sorting = query.getRanking().getSorting();
         List<Sorting.FieldOrder> l = (sorting != null) ? sorting.fieldOrders() : null;
@@ -145,6 +124,11 @@ private ErrorMessage validate(Query query) {
                             f.setSorter(new Sorting.LowerCaseSorter(name));
                         }
                     }
+                    else if (attrConfig.datatype() == AttributesConfig.Attribute.Datatype.TENSOR) {
+                        throw new IllegalArgumentException("Cannot sort on field '" + attrConfig.name() +
+                                                           "' because it is a tensor");
+                    }
+
                 }
                 if (f.getSorter() instanceof Sorting.UcaSorter sorter) {
                     String locale = sorter.getLocale();
@@ -178,4 +162,19 @@ private ErrorMessage validate(Query query) {
         return null;
     }
 
+    private static Sorting.UcaSorter.Strength config2Strength(AttributesConfig.Attribute.Sortstrength.Enum s) {
+        if (s == AttributesConfig.Attribute.Sortstrength.PRIMARY) {
+            return Sorting.UcaSorter.Strength.PRIMARY;
+        } else if (s == AttributesConfig.Attribute.Sortstrength.SECONDARY) {
+            return Sorting.UcaSorter.Strength.SECONDARY;
+        } else if (s == AttributesConfig.Attribute.Sortstrength.TERTIARY) {
+            return Sorting.UcaSorter.Strength.TERTIARY;
+        } else if (s == AttributesConfig.Attribute.Sortstrength.QUATERNARY) {
+            return Sorting.UcaSorter.Strength.QUATERNARY;
+        } else if (s == AttributesConfig.Attribute.Sortstrength.IDENTICAL) {
+            return Sorting.UcaSorter.Strength.IDENTICAL;
+        }
+        return Sorting.UcaSorter.Strength.PRIMARY;
+    }
+
 }

container-search/src/test/java/com/yahoo/prelude/searcher/test/ValidateSortingSearcherTestCase.java

@@ -47,6 +47,17 @@ void testBasicValidation() {
         assertEquals("[ASCENDING:[rank]]", quoteAndTransform("+[relevance]"));
     }
 
+    @Test
+    void testDisallowSortingOnTensors() {
+        try {
+            quoteAndTransform("aTensor");
+            fail("Expected exception");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("Cannot sort on field 'aTensor' because it is a tensor", e.getMessage());
+        }
+    }
+
     @Test
     void testInvalidSpec() {
         assertNull(quoteAndTransform("+a -e +c"));

container-search/src/test/java/com/yahoo/prelude/searcher/test/validate_sorting.cfg

@@ -1,4 +1,4 @@
-attribute[4]
+attribute[5]
 attribute[0].name                title
 attribute[0].datatype            STRING
 attribute[0].collectiontype      SINGLE
@@ -15,3 +15,6 @@ attribute[2].collectiontype      SINGLE
 attribute[3].name                c
 attribute[3].datatype            STRING
 attribute[3].collectiontype      SINGLE
+attribute[4].name                aTensor
+attribute[4].datatype            TENSOR
+attribute[4].collectiontype      SINGLE