Skip to content

Clarify ignore_above behavior with arrays of strings #33057

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

Merged
merged 2 commits into from
Aug 22, 2018

Conversation

dliappis
Copy link
Contributor

and also note that all string(s) will still be visible in the
_source field.

and also note that all string(s) will still be visible in the
`_source` field.
@dliappis dliappis added >docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types labels Aug 22, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Contributor

@colings86 colings86 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 a minor comment but I don't feel strongly about it so happy for this to be merged with or without my suggestion applied

@@ -2,6 +2,9 @@
=== `ignore_above`

Strings longer than the `ignore_above` setting will not be indexed or stored.
For arrays of strings, `ignore_above` will be applied for each array element and string elements longer than `ignore_above` will not be indexed or stored.
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: maybe ...each array element separately and... would be clearer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in eac5614 as per your suggestion, I think it looks clearer now WDYT?

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM

@dliappis
Copy link
Contributor Author

Thanks a lot @colings86 for the blazingly fast review ; thoughts about how far back to go for the backport after CI is green?

@colings86
Copy link
Contributor

I think just back to 6.3 is fine, since these are the active versions

@dliappis dliappis merged commit abb4c18 into elastic:master Aug 22, 2018
dliappis added a commit that referenced this pull request Aug 22, 2018
Currently docs don't explain how `ignore_above` behaves with arrays of
strings.

Clarify how `ignore_above` applies for arrays of strings and
also note that all string(s) will still be visible in the
`_source` field.

Relates #33057
dliappis added a commit that referenced this pull request Aug 22, 2018
Currently docs don't explain how `ignore_above` behaves with arrays of
strings.

Clarify how `ignore_above` applies for arrays of strings and
also note that all string(s) will still be visible in the
`_source` field.

Relates #33057
dliappis added a commit that referenced this pull request Aug 22, 2018
Currently docs don't explain how `ignore_above` behaves with arrays of
strings.

Clarify how `ignore_above` applies for arrays of strings and
also note that all string(s) will still be visible in the
`_source` field.

Relates #33057
@dliappis
Copy link
Contributor Author

Cherry-picked:

@dliappis dliappis deleted the ignore_above_clarifications branch August 22, 2018 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types v6.3.3 v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants