Properly unmarshall keyed Range Aggregation

[astralarya]

The rangeAggregationEntry is currently not exported, which makes the json Marshaller unable to generate a proper json document when marshalling a RangeAggregation.

Fix will be to rename rangeAggregationEntry to RangeAggregationEntry, which should not break any user code since the type is currently unexported and therefore private to the module.

[olivere]

Hi. I'll keep this open and mark it as an enhancement. Marshaling is out of scope for Elastic at the current stage. It's primary use case it to unmarshal and work with results returned from Elasticsearch (see #51 and #64). What is your use case for marshaling data?

[astralarya]

Ohhhh, how embarrasing. So I discovered that the problem is in fact none of these things, but that the SearchHits.Aggregations.Range(string) function is not propertly Unmarshalling the "buckets" object in the response from elasticsearch. The current code seems to expect "buckets" to be an array of AggregationBucketRangeItems objects, while in fact the response is a map of keys to AggregationBucketRangeItems (see https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-range-aggregation.html).

I can write a fix for this, but I've been so off base on what the problem was for this whole time that I just want to run this by you and see what you think first.

[astralarya]

Ah ok, so I think I've got it now. The response where "buckets" is a map only happens when you are making a keyed Range Aggregation. So the right thing to do seems to be to make a SearchHits.Aggregations.KeyedRange(string) function that properly unmarshalls the keyed version of the reponse.

[olivere]

Don't worry. We'll figure that out. It might take some time, but we'll figure it out eventually.

It might be very possible that there's a bug, especially with the Aggregations. Also: While the Elasticsearch documentation is very nice and informative, my trusted source is the Elasticsearch Java source code. While documentation may sometimes lack certain information, fields etc., the source code never lies.

But back to your issue.

Can you write a test that fails, first? It would be nice if you could work with the example mapping/types that I use for all tests (i.e. tweet and comment) that are defined in index_test.go. If we both see what's wrong, I'm sure we can agree on a way to solve the issue. And maybe your PR is the way to go. We'll see.

[olivere]

Hi there. Had some time early in the morning and had a look. I added a test in search_aggs_test.go and used your PR in search_aggs.go. Everything is available in the keyed-aggs branch.

Maybe you can check it out and see if that works for you?

[astralarya]

I tested out your keyed-aggs branch. Everything seems to be working perfectly!

[olivere]

Fine. Merged. Travis is busy and should be ready in a few minutes. Thanks.

[astralarya]

+1 thanks for all the hard work you do on this library!