Add an include_type_name option to 6.x. (#29453)
#37147
Conversation
This adds an `include_type_name` option to the `indices.create`, `indices.get_mapping` and `indices.put_mapping` APIs, which defaults to `true`. When set to `false`, then mappings will be returned directly in the body of the `indices.get_mapping` API, without keying them by the type name, the `indices.create` will expect mappings directly under the `mappings` key, and the `indices.put_mapping` will use `_doc` as a type name and fail if a `type` is provided explicitly. On 5.x indices, get-mapping will fail if the index has multiple mappings, and put-mapping will update or introduce mappings for the `_doc` type instead of updating existing mappings. This oddity is required so that we don't have to introduce a new flag to put-mapping requests to know whether they are actually updating the `_doc` type or performing a typeless call. Relates elastic#15613
jpountz
added
>feature
:Search/Mapping
|
Pinging @elastic/es-search |
There was a problem hiding this comment.
Thanks @jpountz ! I initially had a question about the upgrade path, but deleted it as I was able to resolve it.
| @@ -13,6 +13,10 @@ | |||
| } | |||
| }, | |||
| "params": { | |||
| "include_type_name": { | |||
| "type" : "string", | |||
There was a problem hiding this comment.
Should these be of type "boolean"? I guess we have the same issue on 7.0, so we could fix this separately if you prefer.
There was a problem hiding this comment.
I pushed a commit to this branch to fix this. I also have a PR open on master to apply the same change there.
| @@ -0,0 +1,23 @@ | |||
| --- | |||
| "GET mapping with typeless API on an index that has types": | |||
There was a problem hiding this comment.
Should we also backport the test changes to the standard rest tests (10_basic, 11_basic_with_types, etc.)?
There was a problem hiding this comment.
Also pushed a commit to add these tests.
| @@ -239,6 +241,21 @@ public void refreshMapping(final String index, final String indexUUID) { | |||
| } | |||
| } | |||
|
|
|||
| private DocumentMapper getMapperForUpdate(MapperService mapperService, String type) { | |||
| DocumentMapper mapper = mapperService.documentMapper(type); | |||
| if (mapper == null && type.equals(MapperService.SINGLE_MAPPING_NAME) && | |||
There was a problem hiding this comment.
Instead of checking the index version here, would it make sense to check if the index has multiple mappings? That would be consistent with how typeless 'get mappings' works on indices with multiple types.
There was a problem hiding this comment.
This is what my first version of this PR did but then I had test failures because some tests initially set up a type with a 5.x index and then index documents with the _doc type. It's probably a test bug, but thinking about this case made me unhappy that using the typeless API with a 5.x index would introduce a new type on 6.6 and reuse the existing type on 6.7, which might be surprising. 6.x indices are less problematic in my opinion since typeless API calls were rejected on previous versions, so it's more unlikely to be something that users relied on.
There was a problem hiding this comment.
Another thing to have in mind is that 5.x indices can't be read by Elasticsearch 7.x anyway due to our backward compatibility policy that only supports reading indices created by version N or N-1. So 5.x indices would have to be reindexed prior to upgrading anyway.
There was a problem hiding this comment.
Okay, this reasoning makes sense to me as well.
|
@elasticmachine run gradle build tests 2 |
This adds an
include_type_nameoption to theindices.create,indices.get_mappingandindices.put_mappingAPIs, which defaults totrue.When set to
false, then mappings will be returned directly in the body ofthe
indices.get_mappingAPI, without keying them by the type name, theindices.createwill expect mappings directly under themappingskey, andthe
indices.put_mappingwill use_docas a type name and fail if atypeis provided explicitly.
On 5.x indices, get-mapping will fail if the index has multiple mappings, and
put-mapping will update or introduce mappings for the
_doctype instead ofupdating existing mappings. This oddity is required so that we don't have to
introduce a new flag to put-mapping requests to know whether they are actually
updating the
_doctype or performing a typeless call.Relates #35190