Skip to content

Commit

Permalink
Make sure to use the resolved type in DocumentMapperService#extractMa…
Browse files Browse the repository at this point in the history
…ppings. (#37451)

* Pull out a shared method MapperService#resolveDocumentType.
* Make sure to resolve the type when extracting the mappings.

Addresses #36811.
  • Loading branch information
jtibshirani authored Jan 15, 2019
1 parent 9554b2f commit 1a1dbf7
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 34 deletions.
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

0 comments on commit 1a1dbf7

Please sign in to comment.