From 92856cba3fe5f8a5f0d00db3bfbb2cb4ce3d209e Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Wed, 4 Dec 2019 15:42:50 +0100 Subject: Preserve array updates with element index matching in indexing docproc The resulting `MapValueUpdate` would previously be constructed with the wrong type for the index, causing an exception to be thrown and for the update to fail entirely. --- .../vespa/indexinglanguage/FieldUpdateAdapter.java | 4 +- .../vespa/indexinglanguage/FieldUpdateHelper.java | 10 +++- .../indexinglanguage/DocumentUpdateTestCase.java | 67 +++++++++++++++++++++- .../ValueUpdateToDocumentTestCase.java | 26 +++++++++ 4 files changed, 102 insertions(+), 5 deletions(-) (limited to 'indexinglanguage') diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateAdapter.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateAdapter.java index 7296e4ce61e..bb89ce736f7 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateAdapter.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateAdapter.java @@ -207,7 +207,9 @@ public class FieldUpdateAdapter implements UpdateAdapter { for (Iterator it = arr.fieldValueIterator(); it.hasNext();) { FieldValue childVal = it.next(); for (ValueUpdate childUpd : createValueUpdates(childVal, upd.getUpdate())) { - ret.add(new MapValueUpdate(childVal, childUpd)); + // The array update is always directed towards a particular array index, which is + // kept as the _value_ in the original update. + ret.add(new MapValueUpdate(upd.getValue(), childUpd)); } } return ret; diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateHelper.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateHelper.java index e25af74333d..e51f7984d65 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateHelper.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldUpdateHelper.java @@ -88,7 +88,15 @@ public abstract class FieldUpdateHelper { return val; } else if (upd instanceof MapValueUpdate) { if (val instanceof Array) { - return createFieldValue(val, ((MapValueUpdate)upd).getUpdate()); + var nestedUpdate = ((MapValueUpdate)upd).getUpdate(); + if (nestedUpdate instanceof AssignValueUpdate) { + // Can't assign an array's value type directly to the array, so we have to add it as a + // singular element to the partial document. + ((Array)val).add(nestedUpdate.getValue()); + return val; + } else { + return createFieldValue(val, nestedUpdate); + } } else if (val instanceof MapFieldValue) { throw new UnsupportedOperationException("Can not map into a " + val.getClass().getName() + "."); } else if (val instanceof StructuredFieldValue) { diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/DocumentUpdateTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/DocumentUpdateTestCase.java index beed3053692..a6362e71594 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/DocumentUpdateTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/DocumentUpdateTestCase.java @@ -9,6 +9,7 @@ import com.yahoo.document.update.AddValueUpdate; import com.yahoo.document.update.AssignValueUpdate; import com.yahoo.document.update.FieldUpdate; import com.yahoo.document.update.ValueUpdate; +import com.yahoo.document.update.MapValueUpdate; import com.yahoo.vespa.indexinglanguage.expressions.Expression; import com.yahoo.vespa.indexinglanguage.parser.ParseException; import org.junit.Test; @@ -66,14 +67,19 @@ public class DocumentUpdateTestCase { assertTrue(upd.getCreateIfNonExistent()); } - @Test - public void assign_updates_to_structs_are_preserved() throws ParseException { - var docType = new DocumentType("my_input"); + private static StructDataType makeStructType() { var structType = new StructDataType("foobarstruct"); var fooField = new Field("foo", DataType.STRING); var barField = new Field("bar", DataType.STRING); structType.addField(fooField); structType.addField(barField); + return structType; + } + + @Test + public void assign_updates_to_structs_are_preserved() throws ParseException { + var docType = new DocumentType("my_input"); + var structType = makeStructType(); docType.addField(new Field("mystruct", structType)); var upd = new DocumentUpdate(docType, "id:scheme:my_input::"); @@ -91,4 +97,59 @@ public class DocumentUpdateTestCase { var av = (AssignValueUpdate)valueUpdate; assertEquals(av.getValue(), updatedStruct); } + + @Test + public void assign_matched_array_of_structs_element_update_is_preserved() throws ParseException { + var docType = new DocumentType("my_input"); + var structType = makeStructType(); + var arrayType = ArrayDataType.getArray(structType); + docType.addField(new Field("my_array", arrayType)); + + var updatedStruct = new Struct(structType); + updatedStruct.setFieldValue("foo", new StringFieldValue("new groovy value")); + updatedStruct.setFieldValue("bar", new StringFieldValue("totally tubular!")); + + var upd = new DocumentUpdate(docType, "id:scheme:my_input::"); + var assignUpdate = ValueUpdate.createAssign(updatedStruct); + upd.addFieldUpdate(FieldUpdate.createMap(docType.getField("my_array"), + new IntegerFieldValue(2), assignUpdate)); + + upd = Expression.execute(Expression.fromString("input my_array | passthrough my_array"), upd); + + assertEquals(upd.fieldUpdates().size(), 1); + var fieldUpdate = upd.getFieldUpdate("my_array"); + assertNotNull(fieldUpdate); + var valueUpdate = fieldUpdate.getValueUpdate(0); + assertTrue(valueUpdate instanceof MapValueUpdate); + var mvu = (MapValueUpdate)valueUpdate; + assertEquals(mvu.getValue(), new IntegerFieldValue(2)); + assertEquals(mvu.getUpdate(), assignUpdate); + } + + @Test + public void assign_matched_array_of_primitives_element_update_is_preserved() throws ParseException { + var docType = new DocumentType("my_input"); + var arrayType = ArrayDataType.getArray(DataType.INT); + docType.addField(new Field("my_array", arrayType)); + + var upd = new DocumentUpdate(docType, "id:scheme:my_input::"); + // Use an unreasonably large array index to ensure nothing creates an implicit array under the + // hood when processing the update itself. "Ensure" here means "the test will most likely OOM + // and we'll notice it pretty quickly". + var arrayIndex = new IntegerFieldValue(2_000_000_000); + var assignUpdate = ValueUpdate.createAssign(new IntegerFieldValue(12345)); + upd.addFieldUpdate(FieldUpdate.createMap(docType.getField("my_array"), arrayIndex, assignUpdate)); + + upd = Expression.execute(Expression.fromString("input my_array | passthrough my_array"), upd); + + assertEquals(upd.fieldUpdates().size(), 1); + var fieldUpdate = upd.getFieldUpdate("my_array"); + assertNotNull(fieldUpdate); + var valueUpdate = fieldUpdate.getValueUpdate(0); + assertTrue(valueUpdate instanceof MapValueUpdate); + var mvu = (MapValueUpdate)valueUpdate; + assertEquals(mvu.getValue(), arrayIndex); + assertEquals(mvu.getUpdate(), assignUpdate); + } + } diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ValueUpdateToDocumentTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ValueUpdateToDocumentTestCase.java index b9be7ddbe50..2468dbe5003 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ValueUpdateToDocumentTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ValueUpdateToDocumentTestCase.java @@ -137,6 +137,32 @@ public class ValueUpdateToDocumentTestCase { assertEquals(new IntegerFieldValue(6), arr.get(0)); } + @Test + public void array_of_struct_assign_is_converted() { + DocumentType docType = new DocumentType("my_type"); + StructDataType structType = new StructDataType("my_struct"); + structType.addField(new Field("a", DataType.INT)); + ArrayDataType arrType = DataType.getArray(structType); + Field field = new Field("my_arr", arrType); + docType.addField(field); + + var updatedStruct = new Struct(structType); + updatedStruct.setFieldValue("a", new IntegerFieldValue(42)); + ValueUpdate update = ValueUpdate.createMap(new IntegerFieldValue(2), ValueUpdate.createAssign(updatedStruct)); + + Document doc = FieldUpdateHelper.newPartialDocument(docType, new DocumentId("id:foo:my_type::1"), field, update); + assertNotNull(doc); + // Due to how the roller coaster ride of the partial documents appear to work, we end up creating + // a document with an array that only contains the to-be-updated element, but always at the first + // index rather than the arbitrary updated index (which is good because otherwise it'd have to + // be pre-allocated). + FieldValue obj = doc.getFieldValue("my_arr"); + assertTrue(obj instanceof Array); + Array arr = (Array)obj; + assertEquals(1, arr.size()); + assertEquals(updatedStruct, arr.get(0)); + } + @Test public void requireThatRemoveIsConverted() { DocumentType docType = new DocumentType("my_type"); -- cgit v1.2.3