Skip to content

Convert WildcardFieldMapper to parametrized form#63465

Merged
romseygeek merged 4 commits intoelastic:masterfrom
romseygeek:mapper/wildcard
Oct 13, 2020
Merged

Convert WildcardFieldMapper to parametrized form#63465
romseygeek merged 4 commits intoelastic:masterfrom
romseygeek:mapper/wildcard

Conversation

@romseygeek
Copy link
Copy Markdown
Contributor

Relates to #62988

@romseygeek romseygeek added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.0.0 v7.11.0 labels Oct 8, 2020
@romseygeek romseygeek requested a review from markharwood October 8, 2020 10:58
@romseygeek romseygeek self-assigned this Oct 8, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Oct 8, 2020
@romseygeek romseygeek requested a review from cbuescher October 13, 2020 10:02
@romseygeek
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

Copy link
Copy Markdown
Member

@cbuescher cbuescher 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 two questions for my own education though.

return q instanceof MatchAllDocsQuery || q instanceof MatchAllButRequireVerificationQuery;
}

protected String firstNgramToken(String fragment, Analyzer analyzer) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was this unused?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, IntelliJ flagged it up as never called

throw new MapperParsingException("The field [" + name + "] cannot have indexOptions = " + indexOptions);
}
return this;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious because haven't reviews that many of these moves to ParametrizedFieldMapper yet: These errors woll now happen automatically if no Parameter is defined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, for previously 'global' parameters that don't really make sense on a mapper, you'll get a deprecation warning in 7x and an error in master.

@romseygeek romseygeek merged commit 6db5b5f into elastic:master Oct 13, 2020
@romseygeek romseygeek deleted the mapper/wildcard branch October 13, 2020 13:27
romseygeek added a commit that referenced this pull request Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>refactoring :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v7.11.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants