Merge pull request #24506 from vespa-engine/bratseth/inputs

More input tests and better error message

8 files changed, 107 insertions, 18 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/RankProfile.java b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
index 31c68d2f2c8..cc1c87e6f47 100644
--- a/config-model/src/main/java/com/yahoo/schema/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/schema/RankProfile.java
@@ -783,12 +783,13 @@ public class RankProfile implements Cloneable {
 
         // Combine
         Map<Reference, Input> allInputs = new LinkedHashMap<>();
+
         for (var inheritedProfile : inherited()) {
             for (var input : inheritedProfile.inputs().entrySet()) {
                 Input existing = allInputs.get(input.getKey());
                 if (existing != null && ! existing.equals(input.getValue()))
                     throw new IllegalArgumentException(this + " inherits " + inheritedProfile + " which contains " +
-                                                       input.getValue() + ", but this input is already defined as " +
+                                                       input.getValue() + ", but this is already defined as " +
                                                        existing + " in another profile this inherits");
                 allInputs.put(input.getKey(), input.getValue());
             }
@@ -1403,8 +1404,7 @@ public class RankProfile implements Cloneable {
         @Override
         public boolean equals(Object o) {
             if (o == this) return true;
-            if ( ! (o instanceof Input)) return false;
-            Input other = (Input)o;
+            if ( ! (o instanceof Input other)) return false;
             if ( ! other.name().equals(this.name())) return false;
             if ( ! other.type().equals(this.type())) return false;
             if ( ! other.defaultValue().equals(this.defaultValue())) return false;
@@ -1418,8 +1418,8 @@ public class RankProfile implements Cloneable {
 
         @Override
         public String toString() {
-            return "input '" + name + "' " + type +
-                   (defaultValue().isPresent() ? ":" + defaultValue.get().toAbbreviatedString() : "");
+            return "input " + name + " " + type +
+                   (defaultValue().isPresent() ? ":" + defaultValue.get().toAbbreviatedString(false, true) : "");
         }
 
     }
diff --git a/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java b/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java
index 51d19283b1e..85225f0d255 100644
--- a/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/RankProfileTestCase.java
@@ -24,11 +24,13 @@ import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModels;
 
 import static com.yahoo.config.model.test.TestUtil.joinLines;
 
+import com.yahoo.searchlib.rankingexpression.Reference;
 import org.junit.jupiter.api.Test;
 
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -168,6 +170,74 @@ public class RankProfileTestCase extends AbstractSchemaTestCase {
     }
 
     @Test
+    void inputsAreInheritedAndValuesAreOverridable() throws ParseException {
+        RankProfileRegistry registry = new RankProfileRegistry();
+        ApplicationBuilder builder = new ApplicationBuilder(registry, new QueryProfileRegistry());
+        builder.addSchema(joinLines(
+                "schema test {",
+                "  document test { } ",
+                "  rank-profile parent1 {",
+                "    inputs {",
+                "      input1 double: 1",
+                "      input2 double: 2",
+                "    }",
+                "  }",
+                "  rank-profile parent2 {",
+                "    inputs {",
+                "      input4 double: 4",
+                "    }",
+                "  }",
+                "  rank-profile child inherits parent1, parent2 {",
+                "    inputs {",
+                "      input2 double: 2.5",
+                "      input3 double: 3",
+                "    }" +
+                "  }",
+                "}"));
+        var application = builder.build(true);
+        RankProfile child = registry.get(application.schemas().get("test"), "child");
+        assertEquals(4, child.inputs().size());
+        assertEquals(Set.of(Reference.simple("query", "input1"),
+                            Reference.simple("query", "input2"),
+                            Reference.simple("query", "input3"),
+                            Reference.simple("query", "input4")),
+                            child.inputs().keySet());
+        var input2 = child.inputs().get(Reference.simple("query", "input2"));
+        assertEquals(2.5, input2.defaultValue().get().asDouble(), 0.000000001);
+    }
+
+    @Test
+    void inputConflictsAreDetected() throws ParseException {
+        try {
+            RankProfileRegistry registry = new RankProfileRegistry();
+            ApplicationBuilder builder = new ApplicationBuilder(registry, new QueryProfileRegistry());
+            builder.addSchema(joinLines(
+                    "schema test {",
+                    "  document test { } ",
+                    "  rank-profile parent1 {",
+                    "    inputs {",
+                    "      input1 double: 1",
+                    "    }",
+                    "  }",
+                    "  rank-profile parent2 {",
+                    "    inputs {",
+                    "      input1 tensor(x[100])",
+                    "    }",
+                    "  }",
+                    "  rank-profile child inherits parent1, parent2 {",
+                    "  }",
+                    "}"));
+            builder.build(true);
+            fail("Should have failed");
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("rank profile 'child' inherits rank profile 'parent2' which contains input query(input1) tensor(x[100])" +
+                         ", but this is already defined as input query(input1) tensor():{1.0} in another profile this inherits",
+                         e.getCause().getMessage());
+        }
+    }
+
+    @Test
     void requireThatDefaultInheritingDefaultIsIgnored() throws ParseException {
         RankProfileRegistry registry = new RankProfileRegistry();
         ApplicationBuilder builder = new ApplicationBuilder(registry, setupQueryProfileTypes());
diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json
index 8714285acd8..5d491dcbd4a 100644
--- a/vespajlib/abi-spec.json
+++ b/vespajlib/abi-spec.json
@@ -906,7 +906,7 @@
       "public com.yahoo.tensor.Tensor remove(java.util.Set)",
       "public java.lang.String toString()",
       "public java.lang.String toString(boolean, boolean)",
-      "public java.lang.String toAbbreviatedString()",
+      "public java.lang.String toAbbreviatedString(boolean, boolean)",
       "public boolean equals(java.lang.Object)",
       "public bridge synthetic com.yahoo.tensor.Tensor withType(com.yahoo.tensor.TensorType)"
     ],
@@ -958,7 +958,7 @@
       "public int hashCode()",
       "public java.lang.String toString()",
       "public java.lang.String toString(boolean, boolean)",
-      "public java.lang.String toAbbreviatedString()",
+      "public java.lang.String toAbbreviatedString(boolean, boolean)",
       "public boolean equals(java.lang.Object)"
     ],
     "fields": []
@@ -1051,7 +1051,7 @@
       "public int hashCode()",
       "public java.lang.String toString()",
       "public java.lang.String toString(boolean, boolean)",
-      "public java.lang.String toAbbreviatedString()",
+      "public java.lang.String toAbbreviatedString(boolean, boolean)",
       "public boolean equals(java.lang.Object)",
       "public long denseSubspaceSize()",
       "public static com.yahoo.tensor.TensorType createPartialType(com.yahoo.tensor.TensorType$Value, java.util.List)"
@@ -1240,7 +1240,8 @@
       "public java.util.List smallest()",
       "public abstract java.lang.String toString()",
       "public abstract java.lang.String toString(boolean, boolean)",
-      "public abstract java.lang.String toAbbreviatedString()",
+      "public java.lang.String toAbbreviatedString()",
+      "public abstract java.lang.String toAbbreviatedString(boolean, boolean)",
       "public static java.lang.String toStandardString(com.yahoo.tensor.Tensor, boolean, boolean, long)",
       "public static java.lang.String valueToString(com.yahoo.tensor.Tensor, boolean, long)",
       "public abstract boolean equals(java.lang.Object)",
diff --git a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java
index c4316eb334a..50809ab3ff6 100644
--- a/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java
+++ b/vespajlib/src/main/java/com/yahoo/tensor/IndexedTensor.java
@@ -219,7 +219,9 @@ public abstract class IndexedTensor implements Tensor {
     }
 
     @Override
-    public String toString() { return toString(true, true); }
+    public String toString() {
+        return toString(true, true);
+    }
 
     @Override
     public String toString(boolean withType, boolean shortForms) {
@@ -227,8 +229,8 @@ public abstract class IndexedTensor implements Tensor {
     }
 
     @Override
-    public String toAbbreviatedString() {
-        return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
+    public String toAbbreviatedString(boolean withType, boolean shortForms) {
+        return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
     }
 
     private String toString(boolean withType, boolean shortForms, long maxCells) {
diff --git a/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java
index 946d8fe0f4a..3ea128ffa9f 100644
--- a/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java
+++ b/vespajlib/src/main/java/com/yahoo/tensor/MappedTensor.java
@@ -78,8 +78,8 @@ public class MappedTensor implements Tensor {
     public String toString(boolean withType, boolean shortForms) { return toString(withType, shortForms, Long.MAX_VALUE); }
 
     @Override
-    public String toAbbreviatedString() {
-        return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
+    public String toAbbreviatedString(boolean withType, boolean shortForms) {
+        return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
     }
 
     private String toString(boolean withType, boolean shortForms, long maxCells) {
diff --git a/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java b/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java
index d2fed9b96f9..e7690876434 100644
--- a/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java
+++ b/vespajlib/src/main/java/com/yahoo/tensor/MixedTensor.java
@@ -154,8 +154,8 @@ public class MixedTensor implements Tensor {
     }
 
     @Override
-    public String toAbbreviatedString() {
-        return toString(true, true, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
+    public String toAbbreviatedString(boolean withType, boolean shortForms) {
+        return toString(withType, shortForms, Math.max(2, 10 / (type().dimensions().stream().filter(d -> d.isMapped()).count() + 1)));
     }
 
     private String toString(boolean withType, boolean shortForms, long maxCells) {
diff --git a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java
index 349214ee7f9..2ad3212c424 100644
--- a/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java
+++ b/vespajlib/src/main/java/com/yahoo/tensor/Tensor.java
@@ -325,7 +325,17 @@ public interface Tensor {
     String toString(boolean withType, boolean shortForms);
 
     /** Returns an abbreviated string representation of this tensor suitable for human-readable messages */
-    String toAbbreviatedString();
+    default String toAbbreviatedString() {
+        return toAbbreviatedString(true, true);
+    }
+
+    /**
+     * Returns an abbreviated string representation of this tensor suitable for human-readable messages
+     *
+     * @param withType whether to prefix the value by the type of this
+     * @param shortForms whether to use short forms where applicable, or always using the verbose form
+     */
+    String toAbbreviatedString(boolean withType, boolean shortForms);
 
     /**
      * Call this from toString in implementations to return this tensor on the
diff --git a/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java b/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java
index 920f8512c53..428c8a83b47 100644
--- a/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java
+++ b/vespajlib/src/test/java/com/yahoo/tensor/TensorTestCase.java
@@ -63,9 +63,11 @@ public class TensorTestCase {
     }
 
     @Test
-    public void testToShortString() {
+    public void testToAbbreviatedString() {
         assertEquals("tensor(x[10]):[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]",
                      Tensor.from("tensor(x[10]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]").toAbbreviatedString());
+        assertEquals("[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0]",
+                     Tensor.from("tensor(x[10]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]").toAbbreviatedString(false, true));
         assertEquals("tensor(x[14]):[0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, ...]",
                      Tensor.from("tensor(x[14]):[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]").toAbbreviatedString());
         assertEquals("tensor(d1{},d2{}):{{d1:l1,d2:l1}:6.0, {d1:l1,d2:l2}:6.0, {d1:l1,d2:l3}:6.0, ...}",
@@ -83,6 +85,10 @@ public class TensorTestCase {
                                  " {m:k3,n:k1,x:0}:0, {m:k3,n:k1,x:1}:1, {m:k3,n:k1,x:2}:2}").toAbbreviatedString());
         assertEquals("tensor(m{},x[2],y[2]):{k1:[[0.0, 1.0], [2.0, 3.0]], k2:[[0.0, ...}",
                      Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString());
+        assertEquals("{k1:[[0.0, 1.0], [2.0, 3.0]], k2:[[0.0, ...}",
+                     Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString(false, true));
+        assertEquals("{{m:k1,x:0,y:0}:0.0, {m:k1,x:0,y:1}:1.0, {m:k1,x:1,y:0}:2.0, {m:k1,x:1,y:1}:3.0, {m:k2,x:0,y:0}:0.0, ...}",
+                     Tensor.from("tensor(m{},x[2],y[2]):{k1:[[0,1],[2,3]], k2:[[0,1],[2,3]], k3:[[0,1],[2,3]]}").toAbbreviatedString(false, false));
     }
 
     @Test