Skip to content

Avoid using external values in parent-join and percolator mappers#71834

Merged
romseygeek merged 2 commits intoelastic:masterfrom
romseygeek:mapper/externalfields
Apr 20, 2021
Merged

Avoid using external values in parent-join and percolator mappers#71834
romseygeek merged 2 commits intoelastic:masterfrom
romseygeek:mapper/externalfields

Conversation

@romseygeek
Copy link
Copy Markdown
Contributor

We would like to remove the use of 'external values' in document parsing.
This commit simplifies two of the four places it is currently used, by adding
direct indexValue methods to BinaryFieldMapper and ParentIdFieldMapper.

Relates to #56063

@romseygeek romseygeek added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.0.0 v7.13.0 labels Apr 19, 2021
@romseygeek romseygeek requested a review from jtibshirani April 19, 2021 12:53
@romseygeek romseygeek self-assigned this Apr 19, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Apr 19, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Thanks @romseygeek for looking into this!

throw new UnsupportedOperationException("Cannot directly call parse() on a ParentIdFieldMapper");
}

public void indexValue(ParseContext context, String refId) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small comment, maybe createIdField would be more precise, since we aren't actually indexing values here. A similar thought applies to BinaryFieldMapper#indexValue.

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.

We've been calling the equivalent methods on other field mappers (created for index-time scripts) indexValue - maybe we should change those as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would be in favor of that!

@romseygeek
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@romseygeek romseygeek merged commit f2ac4f9 into elastic:master Apr 20, 2021
@romseygeek romseygeek deleted the mapper/externalfields branch April 20, 2021 11:18
romseygeek added a commit that referenced this pull request Apr 20, 2021
…1834)

We would like to remove the use of 'external values' in document parsing.
This commit simplifies two of the four places it is currently used, by adding
direct indexValue methods to BinaryFieldMapper and ParentIdFieldMapper.

Relates to #56063
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.13.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants