Skip to content
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

Nest 1.0.2 Aggregations - issue supporting different aggrgation types #931

Closed
djnelson9715 opened this issue Sep 7, 2014 · 3 comments
Closed
Assignees

Comments

@djnelson9715
Copy link

So this has been a bit of a head scratcher

.Aggregations( a=> a
  .Terms("searchTerms", at => at
    .Field("searchTerm.raw")
    .Size(100)
    .Aggregations( rs => rs
      .Stats("pageRankStats", prs =>  prs.Field("pageRank"))
      .Percentiles("pageRankMedian",  prm  => prm.Field("pageRank").Percentages(new double[]{50.0}))))

I should expect to see two aggregations for each search term, the "pageRankStats", "pageRankMedian".

Using the following code to access the results

foreach(KeyItem x in ((Bucket)result.Aggs.Aggregations["searchTerms"]).Items)
{
  var stats = x.Stats("pageRankStats");
  var median = x.Percentiles("pageRankMedian");
}

what I am finding is stats is null and median is populated as expected. If however, I can the second aggregation to be a stats aggregation then I am able to access both aggregation results.

Is this not supported?

Thanks in advance

Doug

@gmarz gmarz self-assigned this Sep 11, 2014
@gmarz
Copy link
Contributor

gmarz commented Sep 11, 2014

@djnelson9715,

Head scratcher indeed. This is definitely a bug and should be supported.

Looks like the problem lies in AggregationConverter.GetNestedAggregations(). When there are multiple nested aggregations on the same level, like in your example, only the first is getting parsed and the rest are skipped.

Need to think of a good solution here, but it's a little tricky. Great catch and thanks for reporting this! Stay tuned for a fix...

@djnelson9715
Copy link
Author

Thanks for the reply. I am sure a lot of other folks are going to really
appreciate this fix. Looking forward to it for sure.

Doug

On Thu, Sep 11, 2014 at 2:37 PM, Greg Marzouka [email protected]
wrote:

@djnelson9715 https://github.com/djnelson9715,

Head scratcher indeed. This is definitely a bug and should be supported.

Looks like the problem lies in
AggregationConverter.GetNestedAggregations()
https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Nest/Resolvers/Converters/Aggregations/AggregationConverter.cs#L256.
When there are multiple nested aggregations on the same level, like in your
example, only the first is getting parsed and the rest are skipped.

Need to think of a good solution here, but it's a little tricky. Great
catch and thanks for reporting this! Stay tuned for a fix...


Reply to this email directly or view it on GitHub
#931 (comment)
.

Doug Nelson

Mpdreamz added a commit that referenced this issue Sep 15, 2014
@Mpdreamz
Copy link
Member

Hi @djnelson9715 I think i captured where it went wrong, the percentiles aggregation response parser did not fully read the json stream to its subset completion causing adjacent aggregation not to be parsed correctly.

Huge 👍 for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants