Revert "Remove aggregation's postCollect phase#68615
Conversation
This partially reverts elastic#64016 and and adds elastic#67839 and adds additional tests that would have caught issues with the changes in elastic#64016. It's mostly Nik's code, I am just cleaning things up a bit. Co-authored-by: Nik Everett <nik9000@gmail.com>
|
@elasticmachine update branch |
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
nik9000
left a comment
There was a problem hiding this comment.
LGTM. Good luck with the trickier bit!
I still don't like postCollect because we have to delay it sometimes so it feels wrong. But this fixes the issue and we'll come up with something better later.
|
I don't like this either, but it looks like we need to find a better solution. |
|
It looks like something went wrong with the backport to 7.11 and this doesn't solve the issue on the branch. I've removed the 7.11.2 label because, while the code is in that release, its not doing what we thought it did. |
|
Interesting, it's definitely in v7.11.2: And changes are there |
|
When I dropped the label I had incorrectly thought that we intended this PR to fix a particular bug who's test had suddenly started failing. It wasn't. We were just adding the test to make sure we had it for when we do fix the bug. So it's correct to say that this is in 7.11.2. |
This partially reverts #64016 and and adds #67839 and adds
additional tests that would have caught issues with the changes
in #64016. It's mostly Nik's code, I am just cleaning things up
a bit. This commit only fixes nested aggs. Fixing the children aggs
is more difficult issue.
Relates to #66876
Co-authored-by: Nik Everett nik9000@gmail.com