Skip to content

Conversation

@ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Jan 27, 2022

Allows running queries against _type on 5.x indices as well as returning _type in search results.

Relates #81210

@ywelsch ywelsch added >non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types v8.1.0 labels Jan 27, 2022
@ywelsch ywelsch mentioned this pull request Jan 27, 2022
32 tasks
@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 27, 2022

@elasticmachine run elasticsearch-ci/rest-compatibility (unrelated failure, as usual these days)

@ywelsch ywelsch marked this pull request as ready for review January 27, 2022 10:52
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jan 27, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@ywelsch ywelsch requested a review from romseygeek January 27, 2022 10:52
Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM, I left one question and one nitpick.

@ywelsch ywelsch merged commit ac9f30a into elastic:master Jan 27, 2022
@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 27, 2022

Thanks @romseygeek!

@jtibshirani
Copy link
Contributor

@ywelsch could you give the background on why we're restoring support for searching and returning _type?

@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 27, 2022

Sure, it's for accessing archival data (#81210) where users will have the ability to access their older raw data. Here, _type is part of the "original data" that the user provided to the system (but isn't stored in _source, similar to _id), so this PR adds support for them to be able to retrieve this data and to run basic queries on it.

@jtibshirani
Copy link
Contributor

Got it, so we're adding some minimal handling for _type as this could store valuable data for 5.x indices. One other question -- I assume this re-adds a top-level _type key to the search response. Does this interact okay with 7.x compatibility, which also adds this (https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/search/SearchHit.java#L627-L629)?

@ywelsch
Copy link
Contributor Author

ywelsch commented Jan 27, 2022

Got it, so we're adding some minimal handling for _type as this could store valuable data for 5.x indices. One other question -- I assume this re-adds a top-level _type key to the search response.

Correct. This is handled quite nicely in newer ES versions, which do that (top-level key) automatically for any metadata mapper.

Does this interact okay with 7.x compatibility, which also adds this (https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/search/SearchHit.java#L627-L629)?

Interesting. This interacts badly, as the _type key would be added twice now (the existing RestApiVersion check should have accounted for that possibility). I can follow-up with a small fix here (low priority).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants