honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow copying from a field to another field that belongs to the same nested object. (#26774) 

The previous test was too strict and enforced that the target object was a
parent. It has been relaxed so that fields that belong to the same nested
object can copy to each other.

The commit also improves error handling in case of multi-fields. The current
validation works but may throw confusing error messages since it assumes that
only object fields may introduce dots in fields names while multi fields may
too.

Closes #26763
This commit is contained in:
Adrien Grand 2017-09-25 18:33:26 +02:00 committed by GitHub
parent 25f154b8c6
commit 579cca9adb
2 changed files with 106 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View File

@ -413,7 +413,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
if (indexSettings.getIndexVersionCreated().onOrAfter(Version.V_6_0_0_beta1)) {
validateCopyTo(fieldMappers, fullPathObjectMappers);
validateCopyTo(fieldMappers, fullPathObjectMappers, fieldTypes);
}
if (reason == MergeReason.MAPPING_UPDATE) {
@ -642,14 +642,26 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
}
private static void validateCopyTo(List<FieldMapper> fieldMappers, Map<String, ObjectMapper> fullPathObjectMappers) {
private static void validateCopyTo(List<FieldMapper> fieldMappers, Map<String, ObjectMapper> fullPathObjectMappers,
FieldTypeLookup fieldTypes) {
for (FieldMapper mapper : fieldMappers) {
if (mapper.copyTo() != null && mapper.copyTo().copyToFields().isEmpty() == false) {
String sourceParent = parentObject(mapper.name());
if (sourceParent != null && fieldTypes.get(sourceParent) != null) {
throw new IllegalArgumentException("[copy_to] may not be used to copy from a multi-field: [" + mapper.name() + "]");
}
final String sourceScope = getNestedScope(mapper.name(), fullPathObjectMappers);
for (String copyTo : mapper.copyTo().copyToFields()) {
String copyToParent = parentObject(copyTo);
if (copyToParent != null && fieldTypes.get(copyToParent) != null) {
throw new IllegalArgumentException("[copy_to] may not be used to copy to a multi-field: [" + copyTo + "]");
}
if (fullPathObjectMappers.containsKey(copyTo)) {
throw new IllegalArgumentException("Cannot copy to field [" + copyTo + "] since it is mapped as an object");
}
final String targetScope = getNestedScope(copyTo, fullPathObjectMappers);
checkNestedScopeCompatibility(sourceScope, targetScope);
}
@ -672,7 +684,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
if (source == null || target == null) {
targetIsParentOfSource = target == null;
} else {
targetIsParentOfSource = source.startsWith(target + ".");
targetIsParentOfSource = source.equals(target) || source.startsWith(target + ".");
}
if (targetIsParentOfSource == false) {
throw new IllegalArgumentException(

91
core/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java
View File

@ -568,4 +568,95 @@ public class CopyToMapperTests extends ESSingleNodeTestCase {
assertArrayEquals(expected, actual);
}
public void testCopyToMultiField() throws Exception {
String mapping = jsonBuilder().startObject().startObject("doc")
.startObject("properties")
.startObject("my_field")
.field("type", "keyword")
.field("copy_to", "my_field.bar")
.startObject("fields")
.startObject("bar")
.field("type", "text")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
MapperService mapperService = createIndex("test").mapperService();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> mapperService.merge("doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE, randomBoolean()));
assertEquals("[copy_to] may not be used to copy to a multi-field: [my_field.bar]", e.getMessage());
}
public void testNestedCopyTo() throws Exception {
String mapping = jsonBuilder().startObject().startObject("doc")
.startObject("properties")
.startObject("n")
.field("type", "nested")
.startObject("properties")
.startObject("foo")
.field("type", "keyword")
.field("copy_to", "n.bar")
.endObject()
.startObject("bar")
.field("type", "text")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
MapperService mapperService = createIndex("test").mapperService();
mapperService.merge("doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE, randomBoolean()); // no exception
}
public void testNestedCopyToMultiField() throws Exception {
String mapping = jsonBuilder().startObject().startObject("doc")
.startObject("properties")
.startObject("n")
.field("type", "nested")
.startObject("properties")
.startObject("my_field")
.field("type", "keyword")
.field("copy_to", "n.my_field.bar")
.startObject("fields")
.startObject("bar")
.field("type", "text")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
MapperService mapperService = createIndex("test").mapperService();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> mapperService.merge("doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE, randomBoolean()));
assertEquals("[copy_to] may not be used to copy to a multi-field: [n.my_field.bar]", e.getMessage());
}
public void testCopyFromMultiField() throws Exception {
String mapping = jsonBuilder().startObject().startObject("doc")
.startObject("properties")
.startObject("my_field")
.field("type", "keyword")
.startObject("fields")
.startObject("bar")
.field("type", "text")
.field("copy_to", "my_field.baz")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
MapperService mapperService = createIndex("test").mapperService();
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> mapperService.merge("doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE, randomBoolean()));
assertThat(e.getMessage(),
Matchers.containsString("copy_to in multi fields is not allowed. Found the copy_to in field [bar] " +
"which is within a multi field."));
}
}