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
Add children aggregation #6936
Add children aggregation #6936
Conversation
This aggregation relies on the <<mapping-parent-field,_parent field>> in the mapping. This aggregation has a single option: | ||
* `child_type` - The what child type the buckets in the parent space should be mapped to. | ||
|
||
For example, lets say we have a index of questions and answers. The answer type has the following `_parent` field in the mapping: |
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.
s/lets/let's/
Looks good overall, but I am suspicious that the aggregator does not always do the right thing (see comment in ChildrenAggregator). |
Any reason you use |
@clintongormley Make sense, the context makes clear that it must be a child type. I will change |
Finally got back to this PR. I applied the feedback. Main changes are:
|
"aggs": { | ||
"top-names": { | ||
"terms": { | ||
"field": "owner_display_name", |
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.
wouldn't it be owner.display_name
?
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.
yes it would!
@martijnvg Just did another review. |
@jpountz Thanks for the review! I implemented your feedback and updated the PR. |
@@ -288,6 +290,63 @@ public long globalMaxOrd(IndexSearcher indexSearcher) { | |||
} | |||
} | |||
|
|||
public static class ParentChild extends Bytes implements ReaderContextAware, TopReaderContextAware { |
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.
Can you put in one level higher so that its qualified name will be ValuesSource.Bytes.ParentChild instead of ValuesSource.Bytes.WithOrdinals.ParentChild?
LGTM |
…ckets between parent types and child types using the already builtin parent/child support. Closes elastic#6936
…ckets between parent types and child types using the already builtin parent/child support. Closes #6936
…ckets between parent types and child types using the already builtin parent/child support. Closes #6936
Can we add "Coming in 1.4.0." to the docs page at http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/search-aggregations-bucket-children-aggregation.html ? Just wasted a few hours trying to figure out why this wouldn't work with my 1.3.2 install |
Made PR for doc fix: #7755 |
Sorry for the lost time @afx114, your doc change is in now. |
Add
children
bucket aggregation that is able to map buckets between parent types and child types based on top of the parent/child support. It is the equivalent of the has_child filter/query in the query dsl.Example request:
Example response: