Merge pull request #32935 from vespa-engine/bratseth/position-fields

Remove special handling of position
vespa-engine · Nov 22, 2024 · a5e923d · a5e923d
2 parents dfce711 + f3b3b2c
commit a5e923d
Show file tree

Hide file tree

Showing 12 changed files with 173 additions and 63 deletions.
diff --git a/config-model/src/main/java/com/yahoo/schema/document/GeoPos.java b/config-model/src/main/java/com/yahoo/schema/document/GeoPos.java
@@ -7,9 +7,11 @@
 /**
  * Common utilities for recognizing fields with the built-in "position" datatype,
  * possibly in array form.
+ *
  * @author arnej
  */
 public class GeoPos {
+
     static public boolean isPos(DataType type) {
         return PositionDataType.INSTANCE.equals(type);
     }
@@ -23,4 +25,5 @@ static public boolean isAnyPos(DataType type) {
     static public boolean isPos(ImmutableSDField field)      { return isPos(field.getDataType()); }
     static public boolean isPosArray(ImmutableSDField field) { return isPosArray(field.getDataType()); }
     static public boolean isAnyPos(ImmutableSDField field)   { return isAnyPos(field.getDataType()); }
+
 }
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
@@ -2,14 +2,10 @@
 package com.yahoo.schema.processing;
 
 import com.yahoo.config.application.api.DeployLogger;
-import com.yahoo.document.ArrayDataType;
 import com.yahoo.document.DataType;
-import com.yahoo.document.MapDataType;
-import com.yahoo.document.WeightedSetDataType;
 import com.yahoo.schema.RankProfileRegistry;
 import com.yahoo.schema.Schema;
 import com.yahoo.schema.document.Attribute;
-import com.yahoo.schema.document.GeoPos;
 import com.yahoo.schema.document.SDField;
 import com.yahoo.vespa.documentmodel.SummaryField;
 import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
@@ -144,26 +140,11 @@ public void tryOutputType(Expression expression, String fieldName, DataType valu
             } else {
                 throw new UnsupportedOperationException();
             }
-            if ( ! fieldType.isAssignableFrom(valueType) &&
-                 ! fieldType.isAssignableFrom(createCompatType(valueType))) {
+            if ( ! fieldType.isAssignableFrom(valueType))
                 throw new VerificationException(expression, "Can not assign " + valueType.getName() + " to " + fieldDesc +
                                                             " '" + fieldName + "' which is " + fieldType.getName() + ".");
-            }
         }
 
-        private static DataType createCompatType(DataType origType) {
-            if (origType instanceof ArrayDataType) {
-                return DataType.getArray(createCompatType(origType.getNestedType()));
-            } else if (origType instanceof MapDataType mapType) {
-                return DataType.getMap(createCompatType(mapType.getKeyType()), createCompatType(mapType.getValueType()));
-            } else if (origType instanceof WeightedSetDataType) {
-                return DataType.getWeightedSet(createCompatType(origType.getNestedType()));
-            } else if (GeoPos.isPos(origType)) {
-                return DataType.LONG;
-            } else {
-                return origType;
-            }
-        }
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java b/config-model/src/test/java/com/yahoo/schema/SchemaTestCase.java
@@ -1,31 +1,25 @@
 // 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.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
 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;
 import com.yahoo.schema.processing.OnnxModelTypeResolver;
 import com.yahoo.vespa.configdefinition.IlscriptsConfig;
 import com.yahoo.vespa.configmodel.producers.DocumentManager;
 import com.yahoo.vespa.documentmodel.DocumentSummary;
-import com.yahoo.vespa.indexinglanguage.expressions.AttributeExpression;
-import com.yahoo.vespa.indexinglanguage.expressions.Expression;
-import com.yahoo.vespa.indexinglanguage.expressions.InputExpression;
-import com.yahoo.vespa.indexinglanguage.expressions.ScriptExpression;
-import com.yahoo.vespa.indexinglanguage.expressions.StatementExpression;
 import com.yahoo.vespa.model.test.utils.DeployLoggerStub;
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
 
 import static com.yahoo.config.model.test.TestUtil.joinLines;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * Schema tests that don't depend on files.
@@ -489,7 +483,7 @@ void testInheritingMultipleRankProfilesWithOverlappingConstructsIsDisallowed2()
     }
 
     @Test
-    void testDeriving() throws Exception {
+    void testDerivingHash() throws Exception {
         String schema =
                 """
                 schema page {
@@ -519,6 +513,31 @@ void testDeriving() throws Exception {
         var documentConfig = new DocumentManager().produce(documentManager, new DocumentmanagerConfig.Builder());
     }
 
+    @Test
+    void testDerivingPosition() throws Exception {
+        String schema =
+                """
+                schema place {
+
+                    document place {
+
+                        field location type position {
+                            indexing: attribute
+                        }
+                    }
+                }""";
+        ApplicationBuilder builder = new ApplicationBuilder(new DeployLoggerStub());
+        builder.addSchema(schema);
+        var application = builder.build(false); // validate=false to test config deriving without validation
+        var derived = new DerivedConfiguration(application.schemas().get("place"), application.rankProfileRegistry());
+        var ilConfig = new IlscriptsConfig.Builder();
+        derived.getIndexingScript().getConfig(ilConfig);
+
+        var documentModel = new DocumentModelBuilder();
+        var documentManager = documentModel.build(List.of(application.schemas().get("place")));
+        var documentConfig = new DocumentManager().produce(documentManager, new DocumentmanagerConfig.Builder());
+    }
+
     private void assertInheritedFromParent(Schema schema, RankProfileRegistry rankProfileRegistry) {
         assertEquals("pf1", schema.fieldSets().userFieldSets().get("parent_set").getFieldNames().stream().findFirst().get());
         assertEquals(Stemming.NONE, schema.getStemming());

diff --git a/docprocs/src/test/cfg3/documentmanager.cfg b/docprocs/src/test/cfg3/documentmanager.cfg
@@ -0,0 +1,54 @@
+ignoreundefinedfields false
+usev8geopositions false
+doctype[0].name "document"
+doctype[0].idx 10000
+doctype[0].contentstruct 10001
+doctype[0].primitivetype[0].idx 10002
+doctype[0].primitivetype[0].name "bool"
+doctype[0].primitivetype[1].idx 10003
+doctype[0].primitivetype[1].name "byte"
+doctype[0].primitivetype[2].idx 10004
+doctype[0].primitivetype[2].name "double"
+doctype[0].primitivetype[3].idx 10005
+doctype[0].primitivetype[3].name "float"
+doctype[0].primitivetype[4].idx 10006
+doctype[0].primitivetype[4].name "float16"
+doctype[0].primitivetype[5].idx 10007
+doctype[0].primitivetype[5].name "int"
+doctype[0].primitivetype[6].idx 10008
+doctype[0].primitivetype[6].name "long"
+doctype[0].primitivetype[7].idx 10010
+doctype[0].primitivetype[7].name "predicate"
+doctype[0].primitivetype[8].idx 10011
+doctype[0].primitivetype[8].name "raw"
+doctype[0].primitivetype[9].idx 10012
+doctype[0].primitivetype[9].name "string"
+doctype[0].primitivetype[10].idx 10014
+doctype[0].primitivetype[10].name "uri"
+doctype[0].wsettype[0].idx 10013
+doctype[0].wsettype[0].elementtype 10012
+doctype[0].wsettype[0].createifnonexistent true
+doctype[0].wsettype[0].removeifzero true
+doctype[0].structtype[0].idx 10001
+doctype[0].structtype[0].name "document.header"
+doctype[0].structtype[1].idx 10009
+doctype[0].structtype[1].name "position"
+doctype[0].structtype[1].field[0].name "x"
+doctype[0].structtype[1].field[0].internalid 914677694
+doctype[0].structtype[1].field[0].type 10007
+doctype[0].structtype[1].field[1].name "y"
+doctype[0].structtype[1].field[1].internalid 900009410
+doctype[0].structtype[1].field[1].type 10007
+doctype[1].name "place"
+doctype[1].idx 10015
+doctype[1].inherits[0].idx 10000
+doctype[1].contentstruct 10016
+doctype[1].fieldsets{[document]}.fields[0] "location"
+doctype[1].structtype[0].idx 10016
+doctype[1].structtype[0].name "place.header"
+doctype[1].structtype[0].field[0].name "location"
+doctype[1].structtype[0].field[0].internalid 654337387
+doctype[1].structtype[0].field[0].type 10009
+doctype[1].structtype[0].field[1].name "location_zcurve"
+doctype[1].structtype[0].field[1].internalid 357924406
+doctype[1].structtype[0].field[1].type 10008
diff --git a/docprocs/src/test/cfg3/ilscripts.cfg b/docprocs/src/test/cfg3/ilscripts.cfg
@@ -0,0 +1,6 @@
+maxtermoccurrences 10000
+maxtokenlength 1000
+fieldmatchmaxlength 1000000
+ilscript[0].doctype "place"
+ilscript[0].docfield[0] "location"
+ilscript[0].content[0] "clear_state | guard { input location | zcurve | attribute location_zcurve; }"
diff --git a/docprocs/src/test/java/com/yahoo/docprocs/indexing/IndexingProcessorTestCase.java b/docprocs/src/test/java/com/yahoo/docprocs/indexing/IndexingProcessorTestCase.java
@@ -1,28 +1,17 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.docprocs.indexing;
 
-import com.yahoo.component.provider.ComponentRegistry;
-import com.yahoo.config.subscription.ConfigGetter;
-import com.yahoo.docproc.Processing;
 import com.yahoo.document.Document;
-import com.yahoo.document.DocumentPut;
 import com.yahoo.document.DocumentOperation;
+import com.yahoo.document.DocumentPut;
 import com.yahoo.document.DocumentType;
-import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.DocumentUpdate;
-import com.yahoo.document.config.DocumentmanagerConfig;
+import com.yahoo.document.PositionDataType;
 import com.yahoo.document.datatypes.StringFieldValue;
-import com.yahoo.document.update.AssignValueUpdate;
 import com.yahoo.document.update.FieldUpdate;
-import com.yahoo.document.update.ValueUpdate;
-import com.yahoo.language.simple.SimpleLinguistics;
-import com.yahoo.vespa.configdefinition.IlscriptsConfig;
 import org.junit.Test;
 
-import java.util.List;
-
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -115,7 +104,30 @@ public void testPut() {
     }
 
     @Test
-    public void testPutCfg2() {
+    public void testPutPosition() {
+        // Config of the following schema, derived Nov 2024, by SchemaTestCase.testDerivingPosition in the config-model
+        //
+        //                schema place {
+        //
+        //                    document place {
+        //
+        //                        field location type position {
+        //                            indexing: attribute
+        //                        }
+        //                    }
+        //                }
+        IndexingProcessorTester tester = new IndexingProcessorTester("src/test/cfg3");
+
+        DocumentType inputType = tester.getDocumentType("place");
+        DocumentPut input = new DocumentPut(inputType, "id:ns:place::");
+        input.getDocument().setFieldValue(inputType.getField("location"), PositionDataType.fromString("13;17"));
+
+        Document output = ((DocumentPut)tester.process(input)).getDocument();
+        assertEquals(595L, output.getFieldValue("location_zcurve").getWrappedValue());
+    }
+
+    @Test
+    public void testPutLongHash() {
         // Config of the following schema, derived Nov 2024, by SchemaTestCase.testDeriving in the config-model
         //
         //                schema page {
@@ -135,15 +147,13 @@ public void testPutCfg2() {
         //                }
         IndexingProcessorTester tester = new IndexingProcessorTester("src/test/cfg2");
 
-        {   // Both artist and title are set
-            DocumentType inputType = tester.getDocumentType("page");
-            DocumentPut input = new DocumentPut(inputType, "id:ns:page::");
-            input.getDocument().setFieldValue(inputType.getField("domain"), new StringFieldValue("domain1"));
+        DocumentType inputType = tester.getDocumentType("page");
+        DocumentPut input = new DocumentPut(inputType, "id:ns:page::");
+        input.getDocument().setFieldValue(inputType.getField("domain"), new StringFieldValue("domain1"));
 
-            Document output = ((DocumentPut)tester.process(input)).getDocument();
-            assertEquals("domain1", output.getFieldValue("domain").getWrappedValue());
-            assertEquals(1386505442371493468L, output.getFieldValue("domain_hash").getWrappedValue());
-        }
+        Document output = ((DocumentPut)tester.process(input)).getDocument();
+        assertEquals("domain1", output.getFieldValue("domain").getWrappedValue());
+        assertEquals(1386505442371493468L, output.getFieldValue("domain_hash").getWrappedValue());
     }
 
     @Test

diff --git a/document/src/main/java/com/yahoo/document/datatypes/Struct.java b/document/src/main/java/com/yahoo/document/datatypes/Struct.java
@@ -210,10 +210,12 @@ public int hashCode() {
     public String toString() {
         StringBuilder retVal = new StringBuilder();
         retVal.append("Struct (").append(getDataType()).append("): ");
-        int [] increasing = getInOrder();
+        int[] increasing = getInOrder();
         for (int id : increasing) {
             retVal.append(getDataType().getField(id)).append("=").append(values.get(id)).append(", ");
         }
+        if (increasing.length > 0)
+            retVal.setLength(retVal.length() - 2);
         return retVal.toString();
     }
 

diff --git a/...language/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ZCurveExpression.java b/...language/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ZCurveExpression.java
@@ -39,7 +39,7 @@ private void requireIntegerField(String fieldName, StructDataType struct) {
     @Override
     public DataType setOutputType(DataType output, VerificationContext context) {
         super.setOutputType(DataType.LONG, output, null, context);
-        return null; // There's no 'any' struct
+        return PositionDataType.INSTANCE;
     }
 
     @Override

diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/SimpleTestAdapter.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/SimpleTestAdapter.java
@@ -35,7 +35,7 @@ public SimpleTestAdapter createField(Field field) {
     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.");
+            throw new VerificationException(exp, "Field '" + fieldName + "' not found.");
         return types.get(fieldName);
     }
 

diff --git a/...inglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/InputTestCase.java b/...inglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/InputTestCase.java
@@ -1,15 +1,21 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.indexinglanguage.expressions;
 
-import com.yahoo.document.*;
+import com.yahoo.document.DataType;
+import com.yahoo.document.Document;
+import com.yahoo.document.DocumentType;
+import com.yahoo.document.Field;
+import com.yahoo.document.StructDataType;
 import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.StringFieldValue;
 import com.yahoo.document.datatypes.Struct;
 import com.yahoo.vespa.indexinglanguage.SimpleDocumentAdapter;
 import com.yahoo.vespa.indexinglanguage.SimpleTestAdapter;
 import org.junit.Test;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.fail;
 
 /**
  * @author Simon Thoresen Hult
@@ -40,7 +46,7 @@ public void requireThatExpressionCanBeVerified() {
             new InputExpression("bar").verify(adapter);
             fail();
         } catch (VerificationException e) {
-            assertEquals("Invalid expression 'input bar': Input field 'bar' not found.", e.getMessage());
+            assertEquals("Invalid expression 'input bar': Field 'bar' not found.", e.getMessage());
         }
     }