-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add missing "aggregations" word #28507
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
Conversation
I think that the sentence "they can only be accessed within the scope of the `nested` query, the `nested`/`reverse_nested` aggregations, or nested-inner-hits" is missing the explanation about what are the `nested` and `reverse_nested` terms. And I guess it must be related to the aggregations that exist for nested objects/documents. Could you apply this change in all ElasticSearch documentation versions? In addition, could you please take the issue I created (elastic#28363) about the last paragraph of this same page? I think it is missing a lot of explanations as it is just a short sentence for something that I think it is really important. Thanks.
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
cbuescher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexmorosmarco makes sense to me too, thanks for the correction. I left a tiny comment, but will merge this to the branches currently active branches once this is adressed.
| Because nested documents are indexed as separate documents, they can only be | ||
| accessed within the scope of the `nested` query, the | ||
| `nested`/`reverse_nested`, or <<nested-inner-hits,nested inner hits>>. | ||
| `nested`/`reverse_nested` aggregations, or <<nested-inner-hits,nested inner hits>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd prefer the singular aggregation here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer it in plural to better state that those are 2 different aggregations.
|
Thanks for your quick response. In addition to this PR, could you please push the issue I created (#28363)? |
|
@alexmorosmarco thanks for this PR, I pushed it to master, the current version 6 branches and the last 5.6 release. |
* master: Add a note to the docs that _cat api `help` option cannot be used if an optional url param is used (elastic#28686) Lift error finding utility to exceptions helpers Change "tweet" type to "_doc" (elastic#28690) [Docs] Add missing word in nested.asciidoc (elastic#28507) Simplify the Translog constructor by always expecting an existing translog (elastic#28676) Upgrade t-digest to 3.2 (elastic#28295) (elastic#28305) Add comment explaining lazy declared versions
|
Thanks for merging. |
I think that the sentence "they can only be
accessed within the scope of the
nestedquery, thenested/reverse_nested, or nested-inner-hits" is missing the explanation about what are thenestedandreverse_nestedterms. And I guess it must be related to the aggregations that exist for nested objects/documents.Could you apply this change in all ElasticSearch documentation versions?
In addition, could you please take the issue I created (#28363) about the last paragraph of this same page? I think it is missing a lot of explanations as it is just a short sentence for something that I think it is really important.
Thanks.