-
Notifications
You must be signed in to change notification settings - Fork 25.2k
[Transform] add support for filter aggregation #52483
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
[Transform] add support for filter aggregation #52483
Conversation
Pinging @elastic/ml-core (:ml/Transform) |
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 really like how this recursive parsing and dot delimited naming of fields work so well :).
...src/main/java/org/elasticsearch/xpack/transform/transforms/pivot/AggregationResultUtils.java
Outdated
Show resolved
Hide resolved
78b63e3
to
68aeabe
Compare
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.
minor observation. Otherwise, looks good :). Super excited to have these in transforms!
subAgg.getName(), | ||
getExtractor(subAgg).value( | ||
subAgg, | ||
fieldTypeMap.get(subLookupFieldPrefix + "." + subAgg.getName()), |
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.
It strikes me that supplying fieldType
is no longer necessary.
Anything that desires to know fieldType
could look it up via its own agg name combined with the prefix (if not empty).
run elasticsearch-ci/bwc |
run elasticsearch-ci/bwc |
2 similar comments
run elasticsearch-ci/bwc |
run elasticsearch-ci/bwc |
8f444d7
to
0d8d6d0
Compare
add support for filter aggregations, refactor code for sub-aggregation support in mapping deduction fixes #52151
add support for filter aggregations, refactor code for sub-aggregation support in mapping deduction
fixes #52151
Note: Dear reviewer, please start with 78b63e3