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

Aggregations with integers as key returns null on keyfield in Nest.KeyItem #730

Closed
andersfa87 opened this issue Jun 19, 2014 · 2 comments
Closed

Comments

@andersfa87
Copy link

andersfa87 commented Jun 19, 2014

I have a query that returns some term aggregations, but when adding the terms where the key is a number, the key will be null on the Nest.KeyItem and the DocCount zero. The aggregations with text keys work just fine. If I run the same query in sense, the values look as expected. My guess is it happens during deserialization, but I wasn't able to find the correct spot. Attached is 2 images showing the data.

@jekstrom
Copy link
Contributor

Ran into this myself. As far as I can tell it happens here: https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Nest/Resolvers/Converters/Aggregations/AggregationConverter.cs#L182 (and perhaps in the other Get.. methods). It reads from the reader and gets the key_as_string value and assumes it is doc_count.

I think just checking if reader.ValueType == typeof(string) and reader.Value == keyItem.key, then skipping over the key_as_string property should work.

Actually, checking reader.Path.Contains("key_as_string") will probably work better.

@gmarz
Copy link
Contributor

gmarz commented Jul 9, 2014

@andersfa87 @jekstrom I wasn't able to reproduce the null Key issue, what version of ES and NEST are you running? See this test: https://github.com/elasticsearch/elasticsearch-net/blob/develop/src/Tests/Nest.Tests.Integration/Reproduce/Reproduce730Tests.cs

I was however, able to reproduce the 0 DocCount. @jekstrom is correct, this was due to the fact that the response for term aggs changed in 1.2 and now includes a "key_as_string" property, which can be skipped. I just pushed the above fix- great catch, thank you!

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