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

Fix error when requesting _type from fields option. #68413

Closed
wants to merge 2 commits into from

Conversation

jtibshirani
Copy link
Contributor

For 8.0 indices, the 'fields' option would throw an error when '_type' is
requested. Since '_type' was removed in 8.0, it should instead behave as if the
field didn't exist, where we don't error but return no values.

Closes #68311.

For 8.0 indices, the 'fields' option would throw an error when '_type' is
requested. Since '_type' was removed in 8.0, it should instead behave as if the
field didn't exist, where we don't error but return no values.
@jtibshirani jtibshirani added >bug :Search/Search Search-related issues that do not fall into other categories v8.0.0 labels Feb 2, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Feb 2, 2021
@elasticmachine
Copy link
Collaborator

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

* A field type representing the document type. This will be null for 8.0 indices,
* which don't have a type and do not support using the _type field in searches.
*/
@Nullable private final TypeFieldType typeFieldType;
Copy link
Contributor Author

@jtibshirani jtibshirani Feb 2, 2021

Choose a reason for hiding this comment

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

@romseygeek I was wondering why we still have this logic on 'master' to handle searches on _type? Should it instead be removed completely? My approach here is to allow using _type against 7.x indices, but for 8.x indices act as though the field doesn't exist. I'm not sure this makes sense -- maybe we should just remove this compatibility logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to removing it entirely in master. I think that was actually my original intention and I just never got round to it after I made the latest round of changes to TypeFieldType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, then I'll close this PR and open a new one to remove TypeFieldType entirely.

For context, what is the plan for handling 7.x API compatibility, where I assume _type will be allowed in searches? (I think I've asked you this 3+ times, I just always seem to forget...)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it has changed a few times, @pgomulka will have a better idea of what the current plan is.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jtibshirani We plan to support _type fields when a request was made with compatible API. if it was on a request, or part of a path it would be parsed but the value ignored. If there will be a need to return a _type field on a response, we will return _doc.
It is great you are asking, I am planning to create few example Compatible API for types removal and ask for feedback (I have few stale PRs with examples, can share offline if you like)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @pgomulka -- I will ping you offline to understand the plan. My question is if I can remove support for _type in searches entirely from master, or if I need to leave it in to coordinate with your plans.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discussed offline and agreed that the compatibility layer should continue to support _type within searches. However @pgomulka was fine with us removing this for now, and then later figuring out the best strategy for retaining compatibility (which may involve restoring some pieces).

@mark-vieira
Copy link
Contributor

@elasticmachine update branch

@jtibshirani
Copy link
Contributor Author

Closing in favor of #68564.

@jtibshirani jtibshirani closed this Feb 4, 2021
@jtibshirani jtibshirani deleted the type-field branch March 18, 2021 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fields API] _type is not returned in the response
6 participants