Enabled sorting terms aggregation by aggregations containing nested

[nazarewk]

This change enables ordering terms by containing nested iaggregation inside order path. #16838

The changes are compatible with both master and 5.3 branches.

Only thing I am not entirely sure of is the ValueCountAggregator.metric() change.

PS: It is somehow related to #22303 but I took entirely different (and way less invasive) approach and written the tests.

[elasticmachine]

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?

[jimczi]

@colings86 can you take a look ?

[colings86]

@nazarewk Thank you for your contribution, but I have the same concerns with this PR as I mentioned in the the issue and in @idozorenko 's PR which is that this relies on getting the first aggregator instance int he multi-bucket aggregator wrapper and using it for the sorting. This will have issues when collecting from multiple buckets because the values required for sorting the second third etc. buckets are not in the first aggregator instance but in the other instance's that the multi-bucket aggregator wrapper has created.

[nazarewk]

@colings86 Can you think of any use case for which i could write tests exposing this issue? I am not proficient enough in elasticsearch codebase to conceive one.

I am already using this in my project and I am not getting any improper computations on mixing nested with terms yet.

My first impulse was to make a wrapper around MultiBucketAggregatorWrapper, but SingleBucketAggregator is just alias for Aggregator which is extension of BucketsAggregator and they both have all the methods final. I know Java, but have never used it much so i am not sure if there is any reason for this and i could just simply final and override methods or extract interfaces?

[nazarewk]

I believe I have encountered the error:

{
  "error": {
    "root_cause": [],
    "type": "reduce_search_phase_exception",
    "reason": "[reduce] ",
    "phase": "fetch",
    "grouped": true,
    "failed_shards": [],
    "caused_by": {
      "type": "class_cast_exception",
      "reason": "org.elasticsearch.search.aggregations.bucket.nested.InternalNested cannot be cast to org.elasticsearch.search.aggregations.InternalMultiBucketAggregation"
    }
  },
  "status": 503
}

I tried to make extended_stats_bucket out of nested>terms>reverse_nested>nested>filter._count

[dakrone]

@colings86 ping on this issue, is this still something we should pursue?

[nazarewk]

I would love to pursue this functionality, but I'm not sure when will I be able to.

…

On 15 Aug 2017 9:57 pm, "Lee Hinman" ***@***.***> wrote: @colings86 <https://github.com/colings86> ping on this issue, is this still something we should pursue? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24121 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADVUUKilfdobzJwwVbNHklyniLds6VcPks5sYfg6gaJpZM4M-IRX> .

[colings86]

@dakrone It would be good to find a solution to this but as I mentioned above, I think that the current approach will suffer from issues as I described. I don't know of a way to get around these issues. I hope we will eventually find a solution but I think its going to be more complex and take longer to achieve than this approach

[colings86]

This has now been fixed via #26683 so I'm going to close this PR. Thanks for the contribution @nazarewk