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.

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

Make sure to use the resolved type in DocumentMapperService#extractMappings. #37451

Merged
merged 2 commits into from
Jan 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,41 @@
- match: { _id: $id}
- match: { _version: 1}
- match: { _source: { foo: bar }}

---
"Index call that introduces new field mappings":

- skip:
version: " - 6.99.99"
reason: Typeless APIs were introduced in 7.0.0

- do:
indices.create: # not using include_type_name: false on purpose
index: index
body:
mappings:
not_doc:
properties:
foo:
type: "keyword"
- do:
index:
index: index
id: 2
body: { new_field: value }

- match: { _index: "index" }
- match: { _type: "_doc" }
- match: { _id: "2" }
- match: { _version: 1 }

- do:
get: # using typeful API on purpose
index: index
type: not_doc
id: 2

- match: { _index: "index" }
- match: { _type: "not_doc" }
- match: { _id: "2" }
- match: { _version: 1}
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,48 @@
id: 1

- match: { _source.foo: baz }

---
"Update call that introduces new field mappings":

- skip:
version: " - 6.99.99"
reason: Typeless APIs were introduced in 7.0.0

- do:
indices.create: # not using include_type_name: false on purpose
index: index
body:
mappings:
not_doc:
properties:
foo:
type: "keyword"

- do:
index:
index: index
type: not_doc
id: 1
body: { foo: bar }

- do:
update:
index: index
id: 1
body:
doc:
foo: baz
new_field: value
- do:
get: # using typeful API on purpose
index: index
type: not_doc
id: 1

- match: { _index: "index" }
- match: { _type: "not_doc" }
- match: { _id: "1" }
- match: { _version: 2}
- match: { _source.foo: baz }
- match: { _source.new_field: value }
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,9 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
}
if (mappingType == null) {
mappingType = newMapper.type();
} else if (mappingType.equals(newMapper.type()) == false) {
throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition");
} else if (mappingType.equals(newMapper.type()) == false
&& mapperService.resolveDocumentType(mappingType).equals(newMapper.type()) == false) {
throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition.");
}
}
assert mappingType != null;
Expand All @@ -295,16 +296,12 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
// we use the exact same indexService and metadata we used to validate above here to actually apply the update
final Index index = indexMetaData.getIndex();
final MapperService mapperService = indexMapperServices.get(index);
String typeForUpdate = mappingType; // the type to use to apply the mapping update
if (MapperService.SINGLE_MAPPING_NAME.equals(typeForUpdate)) {
// If the user gave _doc as a special type value or if (s)he is 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`.
DocumentMapper mapper = mapperService.documentMapper();
if (mapper != null) {
typeForUpdate = mapper.type();
}
}

// 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

CompressedXContent existingSource = null;
DocumentMapper existingMapper = mapperService.documentMapper(typeForUpdate);
if (existingMapper != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ private Tuple<String, Map<String, Object>> extractMapping(String type, Map<Strin

String rootName = root.keySet().iterator().next();
Tuple<String, Map<String, Object>> mapping;
if (type == null || type.equals(rootName)) {
if (type == null || type.equals(rootName) || mapperService.resolveDocumentType(type).equals(rootName)) {
mapping = new Tuple<>(rootName, (Map<String, Object>) root.get(rootName));
} else {
mapping = new Tuple<>(type, root);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,23 @@ public DocumentMapper documentMapper(String type) {
return null;
}

/**
* Resolves a type from a mapping-related request into the type that should be used when
* merging and updating mappings.
*
* If the special `_doc` type is provided, then we replace it with the actual type that is
* being used in the mappings. This allows typeless APIs such as 'index' or 'put mappings'
* to work against indices with a custom type name.
*/
public String resolveDocumentType(String type) {
if (MapperService.SINGLE_MAPPING_NAME.equals(type)) {
if (mapper != null) {
return mapper.type();
}
}
return type;
}

/**
* Returns the document mapper created, including a mapping update if the
* type has been dynamically created.
Expand Down
30 changes: 9 additions & 21 deletions server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ private Engine.IndexResult applyIndexOperation(Engine engine, long seqNo, long o
ensureWriteAllowed(origin);
Engine.Index operation;
try {
final String resolvedType = resolveType(sourceToParse.type());
final String resolvedType = mapperService.resolveDocumentType(sourceToParse.type());
final SourceToParse sourceWithResolvedType;
if (resolvedType.equals(sourceToParse.type())) {
sourceWithResolvedType = sourceToParse;
Expand Down Expand Up @@ -844,11 +844,12 @@ private Engine.DeleteResult applyDeleteOperation(Engine engine, long seqNo, long
} catch (MapperParsingException | IllegalArgumentException | TypeMissingException e) {
return new Engine.DeleteResult(e, version, operationPrimaryTerm, seqNo, false);
}
if (resolveType(type).equals(mapperService.documentMapper().type()) == false) {
if (mapperService.resolveDocumentType(type).equals(mapperService.documentMapper().type()) == false) {
// We should never get there due to the fact that we generate mapping updates on deletes,
// but we still prefer to have a hard exception here as we would otherwise delete a
// document in the wrong type.
throw new IllegalStateException("Deleting document from type [" + resolveType(type) + "] while current type is [" +
throw new IllegalStateException("Deleting document from type [" +
mapperService.resolveDocumentType(type) + "] while current type is [" +
mapperService.documentMapper().type() + "]");
}
final Term uid = new Term(IdFieldMapper.NAME, Uid.encodeId(id));
Expand All @@ -861,8 +862,8 @@ private Engine.Delete prepareDelete(String type, String id, Term uid, long seqNo
VersionType versionType, Engine.Operation.Origin origin,
long ifSeqNo, long ifPrimaryTerm) {
long startTime = System.nanoTime();
return new Engine.Delete(resolveType(type), id, uid, seqNo, primaryTerm, version, versionType, origin, startTime,
ifSeqNo, ifPrimaryTerm);
return new Engine.Delete(mapperService.resolveDocumentType(type), id, uid, seqNo, primaryTerm, version, versionType,
origin, startTime, ifSeqNo, ifPrimaryTerm);
}

private Engine.DeleteResult delete(Engine engine, Engine.Delete delete) throws IOException {
Expand All @@ -885,7 +886,7 @@ private Engine.DeleteResult delete(Engine engine, Engine.Delete delete) throws I
public Engine.GetResult get(Engine.Get get) {
readAllowed();
DocumentMapper mapper = mapperService.documentMapper();
if (mapper == null || mapper.type().equals(resolveType(get.type())) == false) {
if (mapper == null || mapper.type().equals(mapperService.resolveDocumentType(get.type())) == false) {
return GetResult.NOT_EXISTS;
}
return getEngine().get(get, this::acquireSearcher);
Expand Down Expand Up @@ -2319,23 +2320,10 @@ private static void persistMetadata(
}
}

/**
* If an index/update/get/delete operation is using the special `_doc` type, then we replace
* it with the actual type that is being used in the mappings so that users may use typeless
* APIs with indices that have types.
*/
private String resolveType(String type) {
if (MapperService.SINGLE_MAPPING_NAME.equals(type)) {
DocumentMapper docMapper = mapperService.documentMapper();
if (docMapper != null) {
return docMapper.type();
}
}
return type;
}

private DocumentMapperForType docMapper(String type) {
return mapperService.documentMapperWithAutoCreate(resolveType(type));
return mapperService.documentMapperWithAutoCreate(
mapperService.resolveDocumentType(type));
}

private EngineConfig newEngineConfig() {
Expand Down