Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat put-mapping calls with _doc as a top-level key as typed calls. #38032

Merged
merged 5 commits into from
Jan 31, 2019
Merged

Treat put-mapping calls with _doc as a top-level key as typed calls. #38032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0669240
Treat put-mapping calls with `_doc` as a top-level key as typed calls.
jpountz Jan 30, 2019
3f18fa7
iter
jpountz Jan 30, 2019
95175a7
Merge branch 'master' into fix/put_mapping_with_doc_type
jpountz Jan 31, 2019
88bcd01
Merge branch 'master' into fix/put_mapping_with_doc_type
jpountz Jan 31, 2019
eaec572
iter
jpountz Jan 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion ...pec/src/main/resources/rest-api-spec/test/indices.put_mapping/20_mix_typeless_typeful.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,45 @@
type: "keyword" # also test no-op updates that trigger special logic wrt the mapping version

- do:
catch: bad_request
catch: /the final mapping would have more than 1 type/
indices.put_mapping:
include_type_name: true
index: index
type: some_other_type
body:
some_other_type:
properties:
bar:
type: "long"


---
"PUT mapping with _doc on an index that has types":

- skip:
version: " - 6.99.99"
reason: Backport first


- do:
indices.create:
include_type_name: true
index: index
body:
mappings:
my_type:
properties:
foo:
type: "keyword"

- do:
catch: /the final mapping would have more than 1 type/
indices.put_mapping:
include_type_name: true
index: index
type: _doc
body:
_doc:
properties:
bar:
type: "long"
25 changes: 20 additions & 5 deletions server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.mapper.DocumentMapper;
Expand Down Expand Up @@ -277,7 +279,8 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
if (mappingType == null) {
mappingType = newMapper.type();
} else if (mappingType.equals(newMapper.type()) == false
&& mapperService.resolveDocumentType(mappingType).equals(newMapper.type()) == false) {
&& (isMappingSourceTyped(mapperService, mappingUpdateSource, request.type())
|| mapperService.resolveDocumentType(mappingType).equals(newMapper.type()) == false)) {
throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition.");
}
}
Expand All @@ -297,10 +300,13 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
final Index index = indexMetaData.getIndex();
final MapperService mapperService = indexMapperServices.get(index);

// If the user gave _doc as a special type value or if they are using the new typeless APIs,
// then we apply the mapping update to the existing type. This allows to move to typeless
// APIs with indices whose type name is different from `_doc`.
String typeForUpdate = mapperService.resolveDocumentType(mappingType); // the type to use to apply the mapping update
// If the _type name is _doc and there is no _doc top-level key then this means that we
// are handling a typeless call. In such a case, we override _doc with the actual type
// name in the mappings. This allows to use typeless APIs on typed indices.
String typeForUpdate = mappingType; // the type to use to apply the mapping update
if (isMappingSourceTyped(mapperService, mappingUpdateSource, request.type()) == false) {
Copy link
Contributor

@jtibshirani jtibshirani Jan 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To check I understand: on an index with a custom type, why does issuing a typeless 'index' request that updates the mappings still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The situation is different on 7.x and 6.x. On 7.x, a typeless index request on an index that has types triggers the creation of a put-mapping request for the actual type of the mapping, so we are good. This is different on 6.x which doesn't have logic to make typeless index requests work on typed indices, so doing it would actually create a typed put-mapping call with _doc as a type name. This is how I found about this issue when backporting the change that deals with merging mappings and templates.

typeForUpdate = mapperService.resolveDocumentType(mappingType);
}

CompressedXContent existingSource = null;
DocumentMapper existingMapper = mapperService.documentMapper(typeForUpdate);
Expand Down Expand Up @@ -365,6 +371,15 @@ public String describeTasks(List<PutMappingClusterStateUpdateRequest> tasks) {
}
}

/**
* Returns {@code true} if the given {@code mappingSource} includes a type
* as a top-level object.
*/
private static boolean isMappingSourceTyped(MapperService mapperService, CompressedXContent mappingSource, String type) {
Map<String, Object> root = XContentHelper.convertToMap(mappingSource.compressedReference(), true, XContentType.JSON).v2();
return root.size() == 1 && root.keySet().iterator().next().equals(type);
}

public void putMapping(final PutMappingClusterStateUpdateRequest request, final ActionListener<ClusterStateUpdateResponse> listener) {
clusterService.submitStateUpdateTask("put-mapping",
request,
Expand Down