Merge pull request #32799 from vespa-engine/bratseth/indexing-type-in…

…ference-take-2

Bratseth/indexing type inference take 2

config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java

@@ -52,7 +52,7 @@ public void process(boolean validate, boolean documentsOnly) {
                     converter.convert(exp); // TODO: stop doing this explicitly when visiting a script does not branch
                 }
             } catch (VerificationException e) {
-                fail(schema, field, "For expression '" + e.getExpression() + "': " + Exceptions.toMessageString(e));
+                fail(schema, field, Exceptions.toMessageString(e));
             }
         }
     }
@@ -71,17 +71,17 @@ public ExpressionConverter branch() {
         }
 
         @Override
-        protected boolean shouldConvert(Expression exp) {
-            if (exp instanceof OutputExpression) {
-                String fieldName = ((OutputExpression)exp).getFieldName();
+        protected boolean shouldConvert(Expression expression) {
+            if (expression instanceof OutputExpression) {
+                String fieldName = ((OutputExpression)expression).getFieldName();
                 if (outputs.contains(fieldName) && !prevNames.contains(fieldName)) {
-                    throw new VerificationException(exp, "Attempting to assign conflicting values to field '" +
-                                                         fieldName + "'");
+                    throw new VerificationException(expression, "Attempting to assign conflicting values to field '" +
+                                                                fieldName + "'");
                 }
                 outputs.add(fieldName);
                 prevNames.add(fieldName);
             }
-            if (exp.createdOutputType() != null) {
+            if (expression.createdOutputType() != null) {
                 prevNames.clear();
             }
             return false;

config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java

@@ -1,7 +1,10 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.schema;
 
+import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.document.Document;
+import com.yahoo.schema.derived.DerivedConfiguration;
+import com.yahoo.schema.derived.SchemaInfo;
 import com.yahoo.schema.document.Stemming;
 import com.yahoo.schema.parser.ParseException;
 import com.yahoo.schema.processing.ImportedFieldsResolver;
@@ -479,6 +482,25 @@ void testInheritingMultipleRankProfilesWithOverlappingConstructsIsDisallowed2()
         }
     }
 
+    @Test
+    void testDeriving() throws Exception {
+        String schema =
+                """
+                schema test {
+                    field my_hash type long {
+                        indexing: input my_string | hash | attribute
+                    }
+                    document test {
+                        field my_string type string {
+                        }
+                    }
+                }""";
+        ApplicationBuilder builder = new ApplicationBuilder(new DeployLoggerStub());
+        builder.addSchema(schema);
+        var application = builder.build(false); // validate=false to test config deriving without validation
+        new DerivedConfiguration(application.schemas().get("test"), application.rankProfileRegistry());
+    }
+
     private void assertInheritedFromParent(Schema schema, RankProfileRegistry rankProfileRegistry) {
         assertEquals("pf1", schema.fieldSets().userFieldSets().get("parent_set").getFieldNames().stream().findFirst().get());
         assertEquals(Stemming.NONE, schema.getStemming());

config-model/src/test/java/com/yahoo/schema/processing/IndexingInputsTestCase.java

@@ -57,7 +57,7 @@ void requireThatExtraFieldInputImplicitThrows() throws ParseException {
         }
         catch (IllegalArgumentException e) {
             assertEquals("For schema 'indexing_extra_field_input_implicit', field 'foo': " +
-                         "For expression '{ tokenize normalize stem:\"BEST\" | index foo; }': Expected string input, but no input is specified",
+                         "Invalid expression '{ tokenize normalize stem:\"BEST\" | index foo; }': Expected string input, but no input is specified",
                          Exceptions.toMessageString(e));
         }
     }
@@ -156,7 +156,7 @@ void testNoInputInDerivedField() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'test', field 'derived1': For expression '{ attribute derived1; }': " +
+            assertEquals("For schema 'test', field 'derived1': Invalid expression '{ attribute derived1; }': " +
                          "Expected any input, but no input is specified",
                          Exceptions.toMessageString(e));
         }

config-model/src/test/java/com/yahoo/schema/processing/IndexingOutputsTestCase.java

@@ -56,7 +56,7 @@ void requireThatOutputConflictThrows() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'indexing_output_confict', field 'bar': For expression 'index bar': Attempting " +
+            assertEquals("For schema 'indexing_output_confict', field 'bar': Invalid expression 'index bar': Attempting " +
                          "to assign conflicting values to field 'bar'",
                          Exceptions.toMessageString(e));
         }

config-model/src/test/java/com/yahoo/schema/processing/IndexingValidationTestCase.java

@@ -35,7 +35,7 @@ void testAttributeChanged() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'indexing_attribute_changed', field 'foo': For expression 'attribute foo': " +
+            assertEquals("For schema 'indexing_attribute_changed', field 'foo': Invalid expression 'attribute foo': " +
                          "Attempting to assign conflicting values to field 'foo'",
                          Exceptions.toMessageString(e));
         }
@@ -79,7 +79,7 @@ void testIndexChanged() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'indexing_index_changed', field 'foo': For expression 'index foo': " +
+            assertEquals("For schema 'indexing_index_changed', field 'foo': Invalid expression 'index foo': " +
                          "Attempting to assign conflicting values to field 'foo'",
                          Exceptions.toMessageString(e));
         }
@@ -123,7 +123,7 @@ void testSummaryChanged() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'indexing_summary_fail', field 'foo': For expression 'summary foo': Attempting " +
+            assertEquals("For schema 'indexing_summary_fail', field 'foo': Invalid expression 'summary foo': Attempting " +
                          "to assign conflicting values to field 'foo'",
                          Exceptions.toMessageString(e));
         }
@@ -185,7 +185,7 @@ void requireThatMultilineOutputConflictThrows() throws ParseException {
             fail("Expected exception");
         }
         catch (IllegalArgumentException e) {
-            assertEquals("For schema 'indexing_multiline_output_confict', field 'cox': For expression 'index cox': " +
+            assertEquals("For schema 'indexing_multiline_output_confict', field 'cox': Invalid expression 'index cox': " +
                          "Attempting to assign conflicting values to field 'cox'",
                          Exceptions.toMessageString(e));
         }

document/abi-spec.json

@@ -182,7 +182,8 @@
       "public com.yahoo.document.datatypes.FieldValue createFieldValue(java.lang.Object)",
       "public abstract java.lang.Class getValueClass()",
       "public abstract boolean isValueCompatible(com.yahoo.document.datatypes.FieldValue)",
-      "public final boolean isAssignableFrom(com.yahoo.document.DataType)",
+      "public boolean isAssignableFrom(com.yahoo.document.DataType)",
+      "public boolean isAssignableTo(com.yahoo.document.DataType)",
       "public static com.yahoo.document.ArrayDataType getArray(com.yahoo.document.DataType)",
       "public static com.yahoo.document.MapDataType getMap(com.yahoo.document.DataType, com.yahoo.document.DataType)",
       "public static com.yahoo.document.WeightedSetDataType getWeightedSet(com.yahoo.document.DataType)",

document/src/main/java/com/yahoo/document/DataType.java

@@ -159,11 +159,16 @@ public FieldValue createFieldValue(Object arg) {
 
     public abstract boolean isValueCompatible(FieldValue value);
 
-    public final boolean isAssignableFrom(DataType dataType) {
+    public boolean isAssignableFrom(DataType dataType) {
         // TODO: Reverse this so that isValueCompatible() uses this instead.
         return isValueCompatible(dataType.createFieldValue());
     }
 
+    /** The reverse of isAssignableFrom */
+    public boolean isAssignableTo(DataType dataType) {
+        return dataType.isAssignableFrom(this);
+    }
+
     /**
      * Returns an array datatype, where the array elements are of the given type
      *

document/src/main/java/com/yahoo/document/NumericDataType.java

@@ -11,14 +11,15 @@ public class NumericDataType extends PrimitiveDataType {
 
     // The global class identifier shared with C++.
     public static int classId = registerClass(Ids.document + 52, NumericDataType.class);
+
     /**
      * Creates a datatype
      *
      * @param name      the name of the type
      * @param code      the code (id) of the type
      * @param type      the field value used for this type
      */
-    protected NumericDataType(java.lang.String name, int code, Class<? extends FieldValue> type, Factory factory) {
+    protected NumericDataType(String name, int code, Class<? extends FieldValue> type, Factory factory) {
         super(name, code, type, factory);
     }
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/AnyDataType.java

@@ -19,12 +19,21 @@ private AnyDataType() {
     }
 
     @Override
-    public FieldValue createFieldValue() { throw new UnsupportedOperationException(); }
+    public boolean isAssignableFrom(DataType other) { return true; }
 
     @Override
-    public Class<?> getValueClass() { throw new UnsupportedOperationException(); }
+    public boolean isAssignableTo(DataType other) {
+        return other instanceof AnyDataType;
+    }
 
     @Override
     public boolean isValueCompatible(FieldValue value) { return true; }
 
+    @Override
+    public FieldValue createFieldValue() { throw new UnsupportedOperationException(); }
+
+    @Override
+    public Class<?> getValueClass() { throw new UnsupportedOperationException(); }
+
+
 }

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ArithmeticExpression.java

@@ -66,11 +66,48 @@ public ArithmeticExpression convertChildren(ExpressionConverter converter) {
         return new ArithmeticExpression(converter.convert(left), op, converter.convert(right));
     }
 
+    @Override
+    public DataType setInputType(DataType inputType, VerificationContext context) {
+        super.setInputType(inputType, context);
+        DataType leftOutput = left.setInputType(inputType, context);
+        DataType rightOutput = right.setInputType(inputType, context);
+        return resultingType(leftOutput, rightOutput);
+    }
+
+    @Override
+    public DataType setOutputType(DataType outputType, VerificationContext context) {
+        super.setOutputType(outputType, context);
+        DataType leftInput = left.setOutputType(outputType, context);
+        DataType rightInput = right.setOutputType(outputType, context);
+        if (leftInput == rightInput) // TODO: Generalize
+            return leftInput;
+        else
+            return getInputType(context);
+    }
+
     @Override
     protected void doVerify(VerificationContext context) {
         DataType input = context.getCurrentType();
-        context.setCurrentType(evaluate(context.setCurrentType(input).verify(left).getCurrentType(),
-                                        context.setCurrentType(input).verify(right).getCurrentType()));
+        context.setCurrentType(resultingType(context.setCurrentType(input).verify(left).getCurrentType(),
+                                             context.setCurrentType(input).verify(right).getCurrentType()));
+    }
+
+    private DataType resultingType(DataType left, DataType right) {
+        if (left == null || right == null)
+            return null;
+        if (!(left instanceof NumericDataType))
+            throw new VerificationException(this, "The first argument must be a number, but has type " + left.getName());
+        if (!(right instanceof NumericDataType))
+            throw new VerificationException(this, "The second argument must be a number, but has type " + right.getName());
+
+        if (left == DataType.FLOAT || left == DataType.DOUBLE || right == DataType.FLOAT || right == DataType.DOUBLE) {
+            if (left == DataType.DOUBLE || right == DataType.DOUBLE)
+                return DataType.DOUBLE;
+            return DataType.FLOAT;
+        }
+        if (left == DataType.LONG || right == DataType.LONG)
+            return DataType.LONG;
+        return DataType.INT;
     }
 
     @Override
@@ -80,15 +117,15 @@ protected void doExecute(ExecutionContext context) {
                                          context.setCurrentValue(input).execute(right).getCurrentValue()));
     }
 
-    private static DataType requiredInputType(Expression lhs, Expression rhs) {
-        DataType lhsType = lhs.requiredInputType();
-        DataType rhsType = rhs.requiredInputType();
-        if (lhsType == null) return rhsType;
-        if (rhsType == null) return lhsType;
-        if (!lhsType.equals(rhsType))
+    private static DataType requiredInputType(Expression left, Expression right) {
+        DataType leftType = left.requiredInputType();
+        DataType rightType = right.requiredInputType();
+        if (leftType == null) return rightType;
+        if (rightType == null) return leftType;
+        if (!leftType.equals(rightType))
             throw new VerificationException(ArithmeticExpression.class, "Operands require conflicting input types, " +
-                                                                        lhsType.getName() + " vs " + rhsType.getName());
-        return lhsType;
+                                                                        leftType.getName() + " vs " + rightType.getName());
+        return leftType;
     }
 
     @Override
@@ -116,23 +153,6 @@ public int hashCode() {
         return getClass().hashCode() + left.hashCode() + op.hashCode() + right.hashCode();
     }
 
-    private DataType evaluate(DataType lhs, DataType rhs) {
-        if (lhs == null || rhs == null)
-            throw new VerificationException(this, "Attempting to perform arithmetic on a null value");
-        if (!(lhs instanceof NumericDataType) || !(rhs instanceof NumericDataType))
-            throw new VerificationException(this, "Attempting to perform unsupported arithmetic: [" +
-                                                  lhs.getName() + "] " + op + " [" + rhs.getName() + "]");
-
-        if (lhs == DataType.FLOAT || lhs == DataType.DOUBLE || rhs == DataType.FLOAT || rhs == DataType.DOUBLE) {
-            if (lhs == DataType.DOUBLE || rhs == DataType.DOUBLE)
-                return DataType.DOUBLE;
-            return DataType.FLOAT;
-        }
-        if (lhs == DataType.LONG || rhs == DataType.LONG)
-            return DataType.LONG;
-        return DataType.INT;
-    }
-
     private FieldValue evaluate(FieldValue lhs, FieldValue rhs) {
         if (lhs == null || rhs == null) return null;
         if (!(lhs instanceof NumericFieldValue) || !(rhs instanceof NumericFieldValue))

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Base64DecodeExpression.java

@@ -24,7 +24,7 @@ public DataType setInputType(DataType inputType, VerificationContext context) {
 
     @Override
     public DataType setOutputType(DataType outputType, VerificationContext context) {
-        super.setOutputType(outputType, DataType.LONG, context);
+        super.setOutputType(DataType.LONG, outputType, null, context);
         return DataType.STRING;
     }
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Base64EncodeExpression.java

@@ -24,7 +24,7 @@ public DataType setInputType(DataType inputType, VerificationContext context) {
 
     @Override
     public DataType setOutputType(DataType outputType, VerificationContext context) {
-        super.setOutputType(outputType, DataType.STRING, context);
+        super.setOutputType(DataType.STRING, outputType, null, context);
         return DataType.LONG;
     }
 

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/BinarizeExpression.java

@@ -38,14 +38,14 @@ public DataType setInputType(DataType inputType, VerificationContext context) {
 
     @Override
     public DataType setOutputType(DataType outputType, VerificationContext context) {
-        return super.setOutputType(outputType, TensorDataType.any(), context);
+        return super.setOutputType(null, outputType, TensorDataType.any(), context);
     }
 
     @Override
     protected void doVerify(VerificationContext context) {
         type = context.getCurrentType();
         if (! (type instanceof TensorDataType))
-            throw new IllegalArgumentException("The 'binarize' function requires a tensor, but got " + type);
+            throw new VerificationException(this, "Require a tensor, but got " + type);
     }
 
     @Override

indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/BusyWaitExpression.java

@@ -41,4 +41,5 @@ private static double nihlakanta(int i) {
     @Override public String toString() { return "busy_wait"; }
     @Override public boolean equals(Object obj) { return obj instanceof BusyWaitExpression; }
     @Override public int hashCode() { return getClass().hashCode(); }
+
 }