Merge pull request #21152 from vespa-engine/geirst/allow-paged-for-more-attribute-types

Allow 'paged' setting for more attribute types.

2 files changed, 96 insertions, 14 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/PagedAttributeValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/PagedAttributeValidator.java
index 2a4f4f18759..2ca4abae2c4 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/PagedAttributeValidator.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/PagedAttributeValidator.java
@@ -9,6 +9,8 @@ import com.yahoo.searchdefinition.document.Attribute;
 import com.yahoo.tensor.TensorType;
 import com.yahoo.vespa.model.container.search.QueryProfiles;
 
+import java.util.Optional;
+
 /**
  * Validates the 'paged' attribute setting and throws if specified on unsupported types.
  *
@@ -38,11 +40,23 @@ public class PagedAttributeValidator extends Processor {
     }
 
     private void validatePagedSetting(Field field, Attribute attribute) {
-        var tensorType = attribute.tensorType();
-        if (tensorType.isEmpty()
-            || !isDenseTensorType(tensorType.get())) {
-            fail(schema, field, "The 'paged' attribute setting is only supported for dense tensor types");
+        if (!isSupportedType(attribute)) {
+            fail(schema, field, "The 'paged' attribute setting is not supported for non-dense tensor, predicate and reference types");
+        }
+    }
+
+    private boolean isSupportedType(Attribute attribute) {
+        var type = attribute.getType();
+        return (type != Attribute.Type.PREDICATE) &&
+                (type != Attribute.Type.REFERENCE) &&
+                (isSupportedTensorType(attribute.tensorType()));
+    }
+
+    private boolean isSupportedTensorType(Optional<TensorType> tensorType) {
+        if (tensorType.isPresent()) {
+            return isDenseTensorType(tensorType.get());
         }
+        return true;
     }
 
     private boolean isDenseTensorType(TensorType type) {
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/PagedAttributeValidatorTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/PagedAttributeValidatorTestCase.java
index 02b5997471b..4eeab15fdd2 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/PagedAttributeValidatorTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/PagedAttributeValidatorTestCase.java
@@ -1,19 +1,70 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.searchdefinition.processing;
 
+import com.yahoo.config.model.application.provider.BaseDeployLogger;
 import com.yahoo.searchdefinition.parser.ParseException;
 import org.junit.Test;
 
+import java.util.Optional;
+
 import static com.yahoo.config.model.test.TestUtil.joinLines;
 import static com.yahoo.searchdefinition.ApplicationBuilder.createFromString;
+import static com.yahoo.searchdefinition.ApplicationBuilder.createFromStrings;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class PagedAttributeValidatorTestCase {
 
     @Test
-    public void dense_tensor_attribute_does_support_paged_setting() throws ParseException {
-        createFromString(getSd("tensor(x[2],y[2])"));
+    public void dense_tensor_attribute_supports_paged_setting() throws ParseException {
+        assertPagedSupported("tensor(x[2],y[2])");
+    }
+
+    @Test
+    public void primitive_attribute_types_support_paged_setting() throws ParseException {
+        assertPagedSupported("int");
+        assertPagedSupported("array<int>");
+        assertPagedSupported("weightedset<int>");
+
+        assertPagedSupported("string");
+        assertPagedSupported("array<string>");
+        assertPagedSupported("weightedset<string>");
+    }
+
+    @Test
+    public void struct_field_attributes_support_paged_setting() throws ParseException {
+        var sd = joinLines("schema test {",
+                "  document test {",
+                "    struct elem {",
+                "      field first type int {}",
+                "      field second type string {}",
+                "    }",
+                "    field foo type array<elem> {",
+                "      indexing: summary",
+                "      struct-field first {",
+                "        indexing: attribute",
+                "        attribute: paged",
+                "      }",
+                "      struct-field second {",
+                "        indexing: attribute",
+                "        attribute: paged",
+                "      }",
+                "    }",
+                "  }",
+                "}");
+
+        var appBuilder = createFromString(sd);
+        var field = appBuilder.getSchema().getField("foo");
+        assertTrue(field.getStructField("first").getAttribute().isPaged());
+        assertTrue(field.getStructField("second").getAttribute().isPaged());
+    }
+
+    private void assertPagedSupported(String fieldType) throws ParseException {
+        var appBuilder = createFromString(getSd(fieldType));
+        var attribute = appBuilder.getSchema().getAttribute("foo");
+        assertTrue(attribute.isPaged());
     }
 
     @Test
@@ -22,25 +73,42 @@ public class PagedAttributeValidatorTestCase {
     }
 
     @Test
-    public void non_tensor_attribute_does_not_support_paged_setting() throws ParseException {
-        assertPagedSettingNotSupported("string");
+    public void predicate_attribute_does_not_support_paged_setting() throws ParseException {
+        assertPagedSettingNotSupported("predicate");
+    }
+
+    @Test
+    public void reference_attribute_does_not_support_paged_setting() throws ParseException {
+        assertPagedSettingNotSupported("reference<parent>", Optional.of(getSd("parent", "int")));
     }
 
     private void assertPagedSettingNotSupported(String fieldType) throws ParseException {
+        assertPagedSettingNotSupported(fieldType, Optional.empty());
+    }
+
+    private void assertPagedSettingNotSupported(String fieldType, Optional<String> parentSd) throws ParseException {
         try {
-            createFromString(getSd(fieldType));
+            if (parentSd.isPresent()) {
+                createFromStrings(new BaseDeployLogger(), parentSd.get(), getSd(fieldType));
+            } else {
+                createFromString(getSd(fieldType));
+            }
             fail("Expected exception");
         }  catch (IllegalArgumentException e) {
-            assertEquals("For schema 'test', field 'foo': The 'paged' attribute setting is only supported for dense tensor types",
+            assertEquals("For schema 'test', field 'foo': The 'paged' attribute setting is not supported for non-dense tensor, predicate and reference types",
                     e.getMessage());
         }
     }
 
-    private String getSd(String type) {
+    private String getSd(String fieldType) {
+        return getSd("test", fieldType);
+    }
+
+    private String getSd(String docType, String fieldType) {
         return joinLines(
-                "schema test {",
-                "  document test {",
-                "    field foo type " + type + "{",
+                "schema " + docType + " {",
+                "  document " + docType + " {",
+                "    field foo type " + fieldType + "{",
                 "      indexing: attribute",
                 "      attribute: paged",
                 "    }",