Merge pull request #32673 from vespa-engine/bratseth/struct-field-type-checking

hmusum · web-flow · commit 16ec113c672f · 2024-10-26T14:25:52.000+02:00
Correct struct field type checking
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java b/config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java
@@ -155,7 +155,7 @@ public void tryOutputType(Expression expression, String fieldName, DataType valu
 
         private static DataType createCompatType(DataType origType) {
             if (origType instanceof ArrayDataType) {
-                return DataType.getArray(createCompatType(((ArrayDataType)origType).getNestedType()));
+                return DataType.getArray(createCompatType(origType.getNestedType()));
             } else if (origType instanceof MapDataType) {
                 MapDataType mapType = (MapDataType)origType;
                 return DataType.getMap(createCompatType(mapType.getKeyType()),
diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GetFieldExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GetFieldExpression.java
@@ -3,48 +3,53 @@
 
 import com.yahoo.document.DataType;
 import com.yahoo.document.Field;
+import com.yahoo.document.StructDataType;
 import com.yahoo.document.StructuredDataType;
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.StructuredFieldValue;
 
 /**
+ * Returns the value of a struct field.
+ *
  * @author Simon Thoresen Hult
  */
 public final class GetFieldExpression extends Expression {
 
-    private final String fieldName;
+    private final String structFieldName;
 
-    public GetFieldExpression(String fieldName) {
+    public GetFieldExpression(String structFieldName) {
         super(UnresolvedDataType.INSTANCE);
-        this.fieldName = fieldName;
+        this.structFieldName = structFieldName;
     }
 
-    public String getFieldName() { return fieldName; }
+    public String getFieldName() { return structFieldName; }
 
     @Override
     public DataType setInputType(DataType inputType, VerificationContext context) {
         super.setInputType(inputType, context);
-        return context.getFieldType(fieldName, this);
+        return getStructFieldType(context);
     }
 
     @Override
     public DataType setOutputType(DataType outputType, VerificationContext context) {
-        super.setOutputType(context.getFieldType(fieldName, this), outputType, context);
+        super.setOutputType(getStructFieldType(context), outputType, context);
         return AnyDataType.instance;
     }
 
     @Override
     protected void doVerify(VerificationContext context) {
+        context.setCurrentType(getStructFieldType(context));
+    }
+
+    private DataType getStructFieldType(VerificationContext context) {
         DataType input = context.getCurrentType();
-        if (!(input instanceof StructuredDataType)) {
+        if ( ! (input instanceof StructuredDataType structInput))
             throw new VerificationException(this, "Expected structured input, got " + input.getName());
-        }
-        Field field = ((StructuredDataType)input).getField(fieldName);
-        if (field == null) {
-            throw new VerificationException(this, "Field '" + fieldName + "' not found in struct type '" +
+        Field field = structInput.getField(structFieldName);
+        if (field == null)
+            throw new VerificationException(this, "Field '" + structFieldName + "' not found in struct type '" +
                                                   input.getName() + "'");
-        }
-        context.setCurrentType(field.getDataType());
+        return field.getDataType();
     }
 
     @Override
@@ -53,9 +58,9 @@ protected void doExecute(ExecutionContext context) {
         if (!(input instanceof StructuredFieldValue struct))
             throw new IllegalArgumentException("Expected structured input, got " + input.getDataType().getName());
 
-        Field field = struct.getField(fieldName);
+        Field field = struct.getField(structFieldName);
         if (field == null)
-            throw new IllegalArgumentException("Field '" + fieldName + "' not found in struct type '" +
+            throw new IllegalArgumentException("Field '" + structFieldName + "' not found in struct type '" +
                                                struct.getDataType().getName() + "'");
         context.setCurrentValue(struct.getFieldValue(field));
     }
@@ -67,19 +72,19 @@ public DataType createdOutputType() {
 
     @Override
     public String toString() {
-        return "get_field " + fieldName;
+        return "get_field " + structFieldName;
     }
 
     @Override
     public boolean equals(Object obj) {
         if (!(obj instanceof GetFieldExpression rhs)) return false;
-        if (!fieldName.equals(rhs.fieldName)) return false;
+        if (!structFieldName.equals(rhs.structFieldName)) return false;
         return true;
     }
 
     @Override
     public int hashCode() {
-        return getClass().hashCode() + fieldName.hashCode();
+        return getClass().hashCode() + structFieldName.hashCode();
     }
 
 }
diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/SimpleTestAdapter.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/SimpleTestAdapter.java
@@ -33,6 +33,9 @@ public SimpleTestAdapter createField(Field field) {
 
     @Override
     public DataType getInputType(Expression exp, String fieldName) {
+        // Same check as in config-model IndexingValidation:
+        if ( ! types.containsKey(fieldName))
+            throw new VerificationException(exp, "Input field '" + fieldName + "' not found.");
         return types.get(fieldName);
     }
 
diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/InputTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/InputTestCase.java
@@ -40,7 +40,7 @@ public void requireThatExpressionCanBeVerified() {
             new InputExpression("bar").verify(adapter);
             fail();
         } catch (VerificationException e) {
-            assertEquals("Field 'bar' not found", e.getMessage());
+            assertEquals("Input field 'bar' not found.", e.getMessage());
         }
     }
 
diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ScriptTestCase.java
@@ -4,9 +4,12 @@
 import com.yahoo.document.ArrayDataType;
 import com.yahoo.document.DataType;
 import com.yahoo.document.Field;
+import com.yahoo.document.StructDataType;
+import com.yahoo.document.datatypes.Array;
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.IntegerFieldValue;
 import com.yahoo.document.datatypes.StringFieldValue;
+import com.yahoo.document.datatypes.Struct;
 import com.yahoo.language.simple.SimpleLinguistics;
 import com.yahoo.vespa.indexinglanguage.SimpleTestAdapter;
 import org.junit.Test;
@@ -146,6 +149,45 @@ public void requireThatVariablesReplaceOthersOutsideScript() {
         assertEquals(new IntegerFieldValue(9), adapter.getInputValue("out"));
     }
 
+    @Test
+    //       indexing: input expressions | for_each {
+    //        get_field myStructField
+    //      } | attribute
+    @SuppressWarnings("unchecked")
+    public void testGetStructField() {
+        var structType = new StructDataType("myStruct");
+        var stringField = new Field("stringField", DataType.STRING);
+        var intField = new Field("intField", DataType.INT); // Nopt accesseed
+        structType.addField(stringField);
+        structType.addField(intField);
+
+        var adapter = new SimpleTestAdapter();
+        adapter.createField(new Field("myInput", new ArrayDataType(structType)));
+        adapter.createField(new Field("myOutput", new ArrayDataType(DataType.STRING)));
+
+        var array = new Array<Struct>(new ArrayDataType(structType));
+        var struct1 = new Struct(structType);
+        struct1.setFieldValue(stringField, "value1");
+        struct1.setFieldValue(intField, 1);
+        array.add(struct1);
+        var struct2 = new Struct(structType);
+        struct2.setFieldValue(stringField, "value2");
+        struct2.setFieldValue(intField, 2);
+        array.add(struct2);
+        adapter.setValue("myInput", array);
+        var statement =
+                newStatement(new InputExpression("myInput"),
+                             new ForEachExpression(new StatementExpression(new GetFieldExpression("stringField"))),
+                             new AttributeExpression("myOutput"));
+        statement.verify(adapter);
+        statement.execute(adapter);
+
+        var result = (Array<StringFieldValue>)adapter.values.get("myOutput");
+        assertEquals(2, result.size());
+        assertEquals("value1", result.get(0).toString());
+        assertEquals("value2", result.get(1).toString());
+    }
+
     @Test
     // input myString | lowercase | summary | index | split ";" | for_each {
     public void testSplitAndForEach() {
diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SelectInputTestCase.java
@@ -60,7 +60,7 @@ public void requireThatExpressionCanBeVerified() {
         assertVerifyThrows(adapter, newSelectInput(new AttributeExpression("my_int"), "my_str"),
                            "Can not assign string to field 'my_int' which is int.");
         assertVerifyThrows(adapter, newSelectInput(new AttributeExpression("my_int"), "my_unknown"),
-                           "Field 'my_unknown' not found");
+                           "Input field 'my_unknown' not found.");
     }
 
     @Test

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,9 @@ public SimpleTestAdapter createField(Field field) {`
`33`	`33`
`34`	`34`	`@Override`
`35`	`35`	`public DataType getInputType(Expression exp, String fieldName) {`
	`36`	`+ // Same check as in config-model IndexingValidation:`
	`37`	`+ if ( ! types.containsKey(fieldName))`
	`38`	`+ throw new VerificationException(exp, "Input field '" + fieldName + "' not found.");`
`36`	`39`	`return types.get(fieldName);`
`37`	`40`	`}`
`38`	`41`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ public void requireThatExpressionCanBeVerified() {`
`40`	`40`	`new InputExpression("bar").verify(adapter);`
`41`	`41`	`fail();`
`42`	`42`	`} catch (VerificationException e) {`
`43`		`- assertEquals("Field 'bar' not found", e.getMessage());`
	`43`	`+ assertEquals("Input field 'bar' not found.", e.getMessage());`
`44`	`44`	`}`
`45`	`45`	`}`
`46`	`46`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ public void requireThatExpressionCanBeVerified() {`
`60`	`60`	`assertVerifyThrows(adapter, newSelectInput(new AttributeExpression("my_int"), "my_str"),`
`61`	`61`	`"Can not assign string to field 'my_int' which is int.");`
`62`	`62`	`assertVerifyThrows(adapter, newSelectInput(new AttributeExpression("my_int"), "my_unknown"),`
`63`		`- "Field 'my_unknown' not found");`
	`63`	`+ "Input field 'my_unknown' not found.");`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`@Test`