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

Remove TypeFieldMapper #53611

Closed

Conversation

romseygeek
Copy link
Contributor

Now that nested documents have their own nested path metadata field in 8x, the
_type field itself is entirely unnecessary. This commit removes the TypeFieldMapper,
and adds some special-case logic to QueryShardContext to return a singleton instance
of TypeFieldType when somebody asks for the _type field to build queries.

@romseygeek romseygeek added >breaking :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0 labels Mar 16, 2020
@romseygeek romseygeek self-assigned this Mar 16, 2020
@elasticmachine
Copy link
Collaborator

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

@romseygeek
Copy link
Contributor Author

@pgomulka @jakelandis I think we'll eventually want to hide the special case logic in QueryShardContext here in a v7 compatibility wrapper so that queries against the _type field are only parsed for v7 clients.

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(QueryShardContext.class));
public static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Using the _type field " +
"in queries and aggregations is deprecated, prefer to use a field instead.";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: If this is only applicable for v7 compatibility mode, should we cal this constant: TYPES_DEPRECATION_MESSAGE_V7?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove _type field from SORTED_META_FIELDS constant?

import org.elasticsearch.search.aggregations.support.ValuesSourceType;

@Deprecated
public final class TypeFieldType extends ConstantFieldType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this class only be used for V7 compatibility mode? may be also worth to put a comment specifying this.

@pgomulka
Copy link
Contributor

if this PR can wait for #53228 to be merged we can use the infrastructure to wrap v7 code and see that type related yml tests from 7.x are still passing

@romseygeek
Copy link
Contributor Author

@pgomulka that's a good idea, we might be able to move things into the compatibility module

@rjernst rjernst added the Team:Search Meta label for search team label May 4, 2020
@romseygeek
Copy link
Contributor Author

This has got very old, I'm going to close and open a new PR.

@romseygeek romseygeek closed this Sep 23, 2020
@romseygeek romseygeek deleted the types-removal/type-field-mapper branch September 23, 2020 17:09
@joegallo
Copy link
Contributor

Related to #62838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants