diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml index 8a27eb3efd219..937c5f19ae5aa 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml @@ -1269,8 +1269,8 @@ synthetic_source with copy_to and ignored values: refresh: true body: name: "B" - k: ["5", "6"] - long: ["7", "8"] + k: ["55", "66"] + long: ["77", "88"] - do: search: @@ -1289,10 +1289,9 @@ synthetic_source with copy_to and ignored values: - match: hits.hits.1._source: name: "B" - k: ["5", "6"] - long: ["7", "8"] - - match: { hits.hits.1.fields.copy: ["5", "6", "7", "8"] } - + k: ["55", "66"] + long: ["77", "88"] + - match: { hits.hits.1.fields.copy: ["55", "66", "77", "88"] } --- synthetic_source with copy_to field having values in source: @@ -1553,3 +1552,402 @@ synthetic_source with copy_to and invalid values for copy: - match: { error.type: "document_parsing_exception" } - contains: { error.reason: "Copy-to currently only works for value-type fields" } + +--- +synthetic_source with copy_to pointing inside object: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + name: + type: keyword + my_values: + properties: + k: + type: keyword + ignore_above: 1 + copy_to: c.copy + long: + type: long + copy_to: c.copy + c: + properties: + copy: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + name: "A" + my_values: + k: "hello" + long: 100 + + - do: + index: + index: test + id: 2 + refresh: true + body: + name: "B" + my_values: + k: ["55", "66"] + long: [77, 88] + + - do: + index: + index: test + id: 3 + refresh: true + body: + name: "C" + my_values: + k: "hello" + long: 100 + c: + copy: "zap" + + - do: + search: + index: test + sort: name + body: + docvalue_fields: [ "c.copy" ] + + - match: + hits.hits.0._source: + name: "A" + my_values: + k: "hello" + long: 100 + - match: + hits.hits.0.fields: + c.copy: [ "100", "hello" ] + + - match: + hits.hits.1._source: + name: "B" + my_values: + k: ["55", "66"] + long: [77, 88] + - match: + hits.hits.1.fields: + c.copy: ["55", "66", "77", "88"] + + - match: + hits.hits.2._source: + name: "C" + my_values: + k: "hello" + long: 100 + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy: [ "100", "hello", "zap" ] + +--- +synthetic_source with copy_to pointing to ambiguous field: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing to ambiguous field and subobjects false: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: false + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing to ambiguous field and subobjects auto: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + subobjects: auto + properties: + k: + type: keyword + copy_to: a.b.c + a: + properties: + b: + properties: + c: + type: keyword + b.c: + type: keyword + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hey" + + - do: + search: + index: test + body: + docvalue_fields: [ "a.b.c" ] + + - match: + hits.hits.0._source: + k: "hey" + - match: + hits.hits.0.fields: + a.b.c: [ "hey" ] + +--- +synthetic_source with copy_to pointing at dynamic field: + - requires: + test_runner_features: contains + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: c.copy + c: + properties: + f: + type: float + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hello" + + - do: + index: + index: test + id: 2 + refresh: true + body: + k: ["55", "66"] + + - do: + index: + index: test + id: 3 + refresh: true + body: + k: "hello" + c: + copy: "zap" + + - do: + search: + index: test + body: + docvalue_fields: [ "c.copy.keyword" ] + + - match: + hits.hits.0._source: + k: "hello" + - match: + hits.hits.0.fields: + c.copy.keyword: [ "hello" ] + + - match: + hits.hits.1._source: + k: ["55", "66"] + - match: + hits.hits.1.fields: + c.copy.keyword: [ "55", "66" ] + + - match: + hits.hits.2._source: + k: "hello" + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy.keyword: [ "hello", "zap" ] + +--- +synthetic_source with copy_to pointing inside dynamic object: + - requires: + cluster_features: ["mapper.source.synthetic_source_copy_to_inside_objects_fix"] + reason: requires copy_to support in synthetic source + + - do: + indices.create: + index: test + body: + mappings: + _source: + mode: synthetic + properties: + k: + type: keyword + copy_to: c.copy + + - do: + index: + index: test + id: 1 + refresh: true + body: + k: "hello" + + - do: + index: + index: test + id: 2 + refresh: true + body: + k: ["55", "66"] + + - do: + index: + index: test + id: 3 + refresh: true + body: + k: "hello" + c: + copy: "zap" + + - do: + search: + index: test + body: + docvalue_fields: [ "c.copy.keyword" ] + + - match: + hits.hits.0._source: + k: "hello" + - match: + hits.hits.0.fields: + c.copy.keyword: [ "hello" ] + + - match: + hits.hits.1._source: + k: ["55", "66"] + - match: + hits.hits.1.fields: + c.copy.keyword: [ "55", "66" ] + + - match: + hits.hits.2._source: + k: "hello" + c: + copy: "zap" + - match: + hits.hits.2.fields: + c.copy.keyword: [ "hello", "zap" ] + diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java index 4d1b68214eddb..c2970d8716147 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java @@ -123,6 +123,7 @@ public int get() { private Field version; private final SeqNoFieldMapper.SequenceIDFields seqID; private final Set fieldsAppliedFromTemplates; + /** * Fields that are copied from values of other fields via copy_to. * This per-document state is needed since it is possible @@ -453,26 +454,16 @@ public boolean isFieldAppliedFromTemplate(String name) { public void markFieldAsCopyTo(String fieldName) { copyToFields.add(fieldName); - if (mappingLookup.isSourceSynthetic() && indexSettings().getSkipIgnoredSourceWrite() == false) { - /* - Mark this field as containing copied data meaning it should not be present - in synthetic _source (to be consistent with stored _source). - Ignored source values take precedence over standard synthetic source implementation - so by adding this nothing entry we "disable" field in synthetic source. - Otherwise, it would be constructed f.e. from doc_values which leads to duplicate values - in copied field after reindexing. - - Note that this applies to fields that are copied from fields using ignored source themselves - and therefore we don't check for canAddIgnoredField(). - */ - ignoredFieldValues.add(IgnoredSourceFieldMapper.NameValue.fromContext(this, fieldName, XContentDataHelper.nothing())); - } } public boolean isCopyToDestinationField(String name) { return copyToFields.contains(name); } + public Set getCopyToFields() { + return copyToFields; + } + /** * Add a new mapper dynamically created while parsing. * diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java index 7b926b091b3a2..d57edb757ba10 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapper.java @@ -26,9 +26,8 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; -import java.util.Comparator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Stream; @@ -113,6 +112,10 @@ NameValue cloneWithValue(BytesRef value) { assert value() == null; return new NameValue(name, parentOffset, value, doc); } + + boolean hasValue() { + return XContentDataHelper.isDataPresent(value); + } } static final class IgnoredValuesFieldMapperType extends StringFieldType { @@ -147,11 +150,38 @@ protected String contentType() { @Override public void postParse(DocumentParserContext context) { // Ignored values are only expected in synthetic mode. - assert context.getIgnoredFieldValues().isEmpty() || context.mappingLookup().isSourceSynthetic(); - List ignoredFieldValues = new ArrayList<>(context.getIgnoredFieldValues()); - // ensure consistent ordering when retrieving synthetic source - Collections.sort(ignoredFieldValues, Comparator.comparing(NameValue::name)); - for (NameValue nameValue : ignoredFieldValues) { + if (context.mappingLookup().isSourceSynthetic() == false) { + assert context.getIgnoredFieldValues().isEmpty(); + return; + } + + Collection ignoredValuesToWrite = context.getIgnoredFieldValues(); + if (context.getCopyToFields().isEmpty() == false && indexSettings.getSkipIgnoredSourceWrite() == false) { + /* + Mark fields as containing copied data meaning they should not be present + in synthetic _source (to be consistent with stored _source). + Ignored source values take precedence over standard synthetic source implementation + so by adding the `XContentDataHelper.voidValue()` entry we disable the field in synthetic source. + Otherwise, it would be constructed f.e. from doc_values which leads to duplicate values + in copied field after reindexing. + */ + var mutableList = new ArrayList<>(ignoredValuesToWrite); + for (String copyToField : context.getCopyToFields()) { + ObjectMapper parent = context.parent().findParentMapper(copyToField); + if (parent == null) { + // There are scenarios when this can happen: + // 1. all values of the field that is the source of copy_to are null + // 2. copy_to points at a field inside a disabled object + // 3. copy_to points at dynamic field which is not yet applied to mapping, we will process it properly on re-parse. + continue; + } + int offset = parent.isRoot() ? 0 : parent.fullPath().length() + 1; + mutableList.add(new IgnoredSourceFieldMapper.NameValue(copyToField, offset, XContentDataHelper.voidValue(), context.doc())); + } + ignoredValuesToWrite = mutableList; + } + + for (NameValue nameValue : ignoredValuesToWrite) { nameValue.doc().add(new StoredField(NAME, encode(nameValue))); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java index 0f224c98241db..d18c3283ef909 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java @@ -40,7 +40,8 @@ public Set getFeatures() { Mapper.SYNTHETIC_SOURCE_KEEP_FEATURE, SourceFieldMapper.SYNTHETIC_SOURCE_WITH_COPY_TO_AND_DOC_VALUES_FALSE_SUPPORT, SourceFieldMapper.SYNTHETIC_SOURCE_COPY_TO_FIX, - FlattenedFieldMapper.IGNORE_ABOVE_SUPPORT + FlattenedFieldMapper.IGNORE_ABOVE_SUPPORT, + SourceFieldMapper.SYNTHETIC_SOURCE_COPY_TO_INSIDE_OBJECTS_FIX ); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index e4ce38d6cec0b..f9c854749e885 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -808,6 +808,42 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep } + ObjectMapper findParentMapper(String leafFieldPath) { + var pathComponents = leafFieldPath.split("\\."); + int startPathComponent = 0; + + ObjectMapper current = this; + String pathInCurrent = leafFieldPath; + + while (current != null) { + if (current.mappers.containsKey(pathInCurrent)) { + return current; + } + + // Go one level down if possible + var parent = current; + current = null; + + var childMapperName = new StringBuilder(); + for (int i = startPathComponent; i < pathComponents.length - 1; i++) { + if (childMapperName.isEmpty() == false) { + childMapperName.append("."); + } + childMapperName.append(pathComponents[i]); + + var childMapper = parent.mappers.get(childMapperName.toString()); + if (childMapper instanceof ObjectMapper objectMapper) { + current = objectMapper; + startPathComponent = i + 1; + pathInCurrent = pathInCurrent.substring(childMapperName.length() + 1); + break; + } + } + } + + return null; + } + protected SourceLoader.SyntheticFieldLoader syntheticFieldLoader(Stream mappers, boolean isFragment) { var fields = mappers.sorted(Comparator.comparing(Mapper::fullPath)) .map(Mapper::syntheticFieldLoader) @@ -828,10 +864,18 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() { private class SyntheticSourceFieldLoader implements SourceLoader.SyntheticFieldLoader { private final List fields; private final boolean isFragment; + private boolean storedFieldLoadersHaveValues; private boolean docValuesLoadersHaveValues; private boolean ignoredValuesPresent; private List ignoredValues; + // If this loader has anything to write. + // In special cases this can be false even if doc values loaders or stored field loaders + // have values. + // F.e. objects that only contain fields that are destinations of copy_to. + private boolean writersHaveValues; + // Use an ordered map between field names and writers to order writing by field name. + private TreeMap currentWriters; private SyntheticSourceFieldLoader(List fields, boolean isFragment) { this.fields = fields; @@ -882,9 +926,55 @@ public boolean advanceToDoc(int docId) throws IOException { } } + @Override + public void prepare() { + if ((storedFieldLoadersHaveValues || docValuesLoadersHaveValues || ignoredValuesPresent) == false) { + writersHaveValues = false; + return; + } + + for (var loader : fields) { + // Currently this logic is only relevant for object loaders. + if (loader instanceof ObjectMapper.SyntheticSourceFieldLoader objectSyntheticFieldLoader) { + objectSyntheticFieldLoader.prepare(); + } + } + + currentWriters = new TreeMap<>(); + + if (ignoredValues != null && ignoredValues.isEmpty() == false) { + for (IgnoredSourceFieldMapper.NameValue value : ignoredValues) { + if (value.hasValue()) { + writersHaveValues |= true; + } + + var existing = currentWriters.get(value.name()); + if (existing == null) { + currentWriters.put(value.name(), new FieldWriter.IgnoredSource(value)); + } else if (existing instanceof FieldWriter.IgnoredSource isw) { + isw.mergeWith(value); + } + } + } + + for (SourceLoader.SyntheticFieldLoader field : fields) { + if (field.hasValue()) { + if (currentWriters.containsKey(field.fieldName()) == false) { + writersHaveValues |= true; + currentWriters.put(field.fieldName(), new FieldWriter.FieldLoader(field)); + } else { + // Skip if the field source is stored separately, to avoid double-printing. + // Make sure to reset the state of loader so that values stored inside will not + // be used after this document is finished. + field.reset(); + } + } + } + } + @Override public boolean hasValue() { - return storedFieldLoadersHaveValues || docValuesLoadersHaveValues || ignoredValuesPresent; + return writersHaveValues; } @Override @@ -892,12 +982,13 @@ public void write(XContentBuilder b) throws IOException { if (hasValue() == false) { return; } + if (isRoot() && isEnabled() == false) { // If the root object mapper is disabled, it is expected to contain // the source encapsulated within a single ignored source value. assert ignoredValues.size() == 1 : ignoredValues.size(); XContentDataHelper.decodeAndWrite(b, ignoredValues.get(0).value()); - ignoredValues = null; + softReset(); return; } @@ -907,41 +998,12 @@ public void write(XContentBuilder b) throws IOException { b.startObject(leafName()); } - if (ignoredValues != null && ignoredValues.isEmpty() == false) { - // Use an ordered map between field names and writer functions, to order writing by field name. - Map orderedFields = new TreeMap<>(); - for (IgnoredSourceFieldMapper.NameValue value : ignoredValues) { - var existing = orderedFields.get(value.name()); - if (existing == null) { - orderedFields.put(value.name(), new FieldWriter.IgnoredSource(value)); - } else if (existing instanceof FieldWriter.IgnoredSource isw) { - isw.mergeWith(value); - } - } - for (SourceLoader.SyntheticFieldLoader field : fields) { - if (field.hasValue()) { - if (orderedFields.containsKey(field.fieldName()) == false) { - orderedFields.put(field.fieldName(), new FieldWriter.FieldLoader(field)); - } else { - // Skip if the field source is stored separately, to avoid double-printing. - // Make sure to reset the state of loader so that values stored inside will not - // be used after this document is finished. - field.reset(); - } - } - } - - for (var writer : orderedFields.values()) { + for (var writer : currentWriters.values()) { + if (writer.hasValue()) { writer.writeTo(b); } - ignoredValues = null; - } else { - for (SourceLoader.SyntheticFieldLoader field : fields) { - if (field.hasValue()) { - field.write(b); - } - } } + b.endObject(); softReset(); } @@ -957,6 +1019,8 @@ private void softReset() { storedFieldLoadersHaveValues = false; docValuesLoadersHaveValues = false; ignoredValuesPresent = false; + ignoredValues = null; + writersHaveValues = false; } @Override @@ -986,34 +1050,49 @@ public String fieldName() { interface FieldWriter { void writeTo(XContentBuilder builder) throws IOException; + boolean hasValue(); + record FieldLoader(SourceLoader.SyntheticFieldLoader loader) implements FieldWriter { @Override public void writeTo(XContentBuilder builder) throws IOException { loader.write(builder); } + + @Override + public boolean hasValue() { + return loader.hasValue(); + } } class IgnoredSource implements FieldWriter { private final String fieldName; private final String leafName; - private final List values; + private final List encodedValues; IgnoredSource(IgnoredSourceFieldMapper.NameValue initialValue) { this.fieldName = initialValue.name(); this.leafName = initialValue.getFieldName(); - this.values = new ArrayList<>(); - this.values.add(initialValue.value()); + this.encodedValues = new ArrayList<>(); + if (initialValue.hasValue()) { + this.encodedValues.add(initialValue.value()); + } } @Override public void writeTo(XContentBuilder builder) throws IOException { - XContentDataHelper.writeMerged(builder, leafName, values); + XContentDataHelper.writeMerged(builder, leafName, encodedValues); + } + + @Override + public boolean hasValue() { + return encodedValues.isEmpty() == false; } public FieldWriter mergeWith(IgnoredSourceFieldMapper.NameValue nameValue) { assert Objects.equals(nameValue.name(), fieldName) : "IgnoredSource is merged with wrong field data"; - - values.add(nameValue.value()); + if (nameValue.hasValue()) { + encodedValues.add(nameValue.value()); + } return this; } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index 3318595ed7129..118cdbffc5db9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -48,6 +48,9 @@ public class SourceFieldMapper extends MetadataFieldMapper { "mapper.source.synthetic_source_with_copy_to_and_doc_values_false" ); public static final NodeFeature SYNTHETIC_SOURCE_COPY_TO_FIX = new NodeFeature("mapper.source.synthetic_source_copy_to_fix"); + public static final NodeFeature SYNTHETIC_SOURCE_COPY_TO_INSIDE_OBJECTS_FIX = new NodeFeature( + "mapper.source.synthetic_source_copy_to_inside_objects_fix" + ); public static final String NAME = "_source"; public static final String RECOVERY_SOURCE_NAME = "_recovery_source"; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java index baff3835d104b..ec255a53e7c5a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java @@ -209,6 +209,9 @@ public void write(LeafStoredFieldLoader storedFieldLoader, int docId, XContentBu if (docValuesLoader != null) { docValuesLoader.advanceToDoc(docId); } + + loader.prepare(); + // TODO accept a requested xcontent type if (loader.hasValue()) { loader.write(b); @@ -299,6 +302,16 @@ public String fieldName() { */ DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException; + /** + Perform any preprocessing needed before producing synthetic source + and deduce whether this mapper (and its children, if any) have values to write. + The expectation is for this method to be called before {@link SyntheticFieldLoader#hasValue()} + and {@link SyntheticFieldLoader#write(XContentBuilder)} are used. + */ + default void prepare() { + // Noop + } + /** * Has this field loaded any values for this document? */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java b/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java index 354f0ec92b0fa..8bacaf8505f91 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/XContentDataHelper.java @@ -72,7 +72,7 @@ public static BytesRef encodeXContentBuilder(XContentBuilder builder) throws IOE } /** - * Returns a special encoded value that signals that values of this field + * Returns a special encoded value that signals that this field * should not be present in synthetic source. * * An example is a field that has values copied to it using copy_to. @@ -80,7 +80,7 @@ public static BytesRef encodeXContentBuilder(XContentBuilder builder) throws IOE * synthetic _source same as it wouldn't be present in stored source. * @return */ - public static BytesRef nothing() { + public static BytesRef voidValue() { return new BytesRef(new byte[] { VOID_ENCODING }); } @@ -112,41 +112,27 @@ static void decodeAndWrite(XContentBuilder b, BytesRef r) throws IOException { /** * Writes encoded values to provided builder. If there are multiple values they are merged into * a single resulting array. + * + * Note that this method assumes all encoded parts have values that need to be written (are not VOID encoded). * @param b destination * @param fieldName name of the field that is written * @param encodedParts subset of field data encoded using methods of this class. Can contain arrays which will be flattened. * @throws IOException */ static void writeMerged(XContentBuilder b, String fieldName, List encodedParts) throws IOException { - var partsWithData = 0; - for (BytesRef encodedPart : encodedParts) { - if (isDataPresent(encodedPart)) { - partsWithData++; - } - } - - if (partsWithData == 0) { + if (encodedParts.isEmpty()) { return; } - if (partsWithData == 1) { + if (encodedParts.size() == 1) { b.field(fieldName); - for (BytesRef encodedPart : encodedParts) { - if (isDataPresent(encodedPart)) { - XContentDataHelper.decodeAndWrite(b, encodedPart); - } - } - + XContentDataHelper.decodeAndWrite(b, encodedParts.get(0)); return; } b.startArray(fieldName); for (var encodedValue : encodedParts) { - if (isDataPresent(encodedValue) == false) { - continue; - } - Optional encodedXContentType = switch ((char) encodedValue.bytes[encodedValue.offset]) { case CBOR_OBJECT_ENCODING, JSON_OBJECT_ENCODING, YAML_OBJECT_ENCODING, SMILE_OBJECT_ENCODING -> Optional.of( getXContentType(encodedValue) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java index 0e97d2b3b46ae..eaa7bf6528203 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java @@ -1518,6 +1518,37 @@ public void testStoredNestedSubObjectWithNameOverlappingParentName() throws IOEx {"path":{"at":{"foo":"A"}}}""", syntheticSource); } + public void testCopyToLogicInsideObject() throws IOException { + DocumentMapper documentMapper = createMapperService(syntheticSourceMapping(b -> { + b.startObject("path"); + b.startObject("properties"); + { + b.startObject("at").field("type", "keyword").field("copy_to", "copy_top.copy").endObject(); + } + b.endObject(); + b.endObject(); + b.startObject("copy_top"); + b.startObject("properties"); + { + b.startObject("copy").field("type", "keyword").endObject(); + } + b.endObject(); + b.endObject(); + })).documentMapper(); + + CheckedConsumer document = b -> { + b.startObject("path"); + b.field("at", "A"); + b.endObject(); + }; + + var doc = documentMapper.parse(source(document)); + assertNotNull(doc.docs().get(0).getField("copy_top.copy")); + + var syntheticSource = syntheticSource(documentMapper, document); + assertEquals("{\"path\":{\"at\":\"A\"}}", syntheticSource); + } + protected void validateRoundTripReader(String syntheticSource, DirectoryReader reader, DirectoryReader roundTripReader) throws IOException { // We exclude ignored source field since in some cases it contains an exact copy of a part of document source. diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java index 8ba57824cf434..3312c94e8a0e1 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java @@ -811,4 +811,40 @@ public void testFlattenExplicitSubobjectsTrue() { exception.getMessage() ); } + + public void testFindParentMapper() { + MapperBuilderContext rootContext = MapperBuilderContext.root(false, false); + + var rootBuilder = new RootObjectMapper.Builder("_doc", Optional.empty()); + rootBuilder.add(new KeywordFieldMapper.Builder("keyword", IndexVersion.current())); + + var child = new ObjectMapper.Builder("child", Optional.empty()); + child.add(new KeywordFieldMapper.Builder("keyword2", IndexVersion.current())); + child.add(new KeywordFieldMapper.Builder("keyword.with.dot", IndexVersion.current())); + var secondLevelChild = new ObjectMapper.Builder("child2", Optional.empty()); + secondLevelChild.add(new KeywordFieldMapper.Builder("keyword22", IndexVersion.current())); + child.add(secondLevelChild); + rootBuilder.add(child); + + var childWithDot = new ObjectMapper.Builder("childwith.dot", Optional.empty()); + childWithDot.add(new KeywordFieldMapper.Builder("keyword3", IndexVersion.current())); + childWithDot.add(new KeywordFieldMapper.Builder("keyword4.with.dot", IndexVersion.current())); + rootBuilder.add(childWithDot); + + RootObjectMapper root = rootBuilder.build(rootContext); + + assertEquals("_doc", root.findParentMapper("keyword").fullPath()); + assertNull(root.findParentMapper("aa")); + + assertEquals("child", root.findParentMapper("child.keyword2").fullPath()); + assertEquals("child", root.findParentMapper("child.keyword.with.dot").fullPath()); + assertNull(root.findParentMapper("child.long")); + assertNull(root.findParentMapper("child.long.hello")); + assertEquals("child.child2", root.findParentMapper("child.child2.keyword22").fullPath()); + + assertEquals("childwith.dot", root.findParentMapper("childwith.dot.keyword3").fullPath()); + assertEquals("childwith.dot", root.findParentMapper("childwith.dot.keyword4.with.dot").fullPath()); + assertNull(root.findParentMapper("childwith.dot.long")); + assertNull(root.findParentMapper("childwith.dot.long.hello")); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java index 4de02178beec1..f4e114da1fa51 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java @@ -240,64 +240,6 @@ private void testWriteMergedWithMixedValues(Object value, List multipleV assertEquals(expected, map.get("foo")); } - public void testWriteMergedWithVoidValue() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - XContentDataHelper.writeMerged(destination, "field", List.of(XContentDataHelper.nothing())); - - destination.endObject(); - - assertEquals("{}", Strings.toString(destination)); - } - - public void testWriteMergedWithMultipleVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.nothing(), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{}", Strings.toString(destination)); - } - - public void testWriteMergedWithMixedVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - var value = XContentFactory.contentBuilder(XContentType.JSON).value(34); - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.encodeXContentBuilder(value), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{\"field\":34}", Strings.toString(destination)); - } - - public void testWriteMergedWithArraysAndVoidValues() throws IOException { - var destination = XContentFactory.contentBuilder(XContentType.JSON); - destination.startObject(); - - var value = XContentFactory.contentBuilder(XContentType.JSON).value(List.of(3, 4)); - XContentDataHelper.writeMerged( - destination, - "field", - List.of(XContentDataHelper.nothing(), XContentDataHelper.encodeXContentBuilder(value), XContentDataHelper.nothing()) - ); - - destination.endObject(); - - assertEquals("{\"field\":[3,4]}", Strings.toString(destination)); - } - private Map executeWriteMergedOnRepeated(Object value) throws IOException { return executeWriteMergedOnTwoEncodedValues(value, value); }