Elasticsearch-net: BucketAggregateBase is not a BucketBase

Created on 15 Oct 2019  路  3Comments  路  Source: elastic/elasticsearch-net

Nest version: 7.3.0

We have a quite simple aggregation that could be really straight or date histogram-based.
Here just example:

```
var result = bla bla bla.Aggregations(pPerformanceAgg => {
var mainAggregation = new AggregationContainerDescriptor

()
.Sum("v", vTerm => vTerm.Field(p => p.v_count))
.Terms("r", rTerm => rTerm.Field(p => p.r) ));

                    if(dateAggregation == DateAggregation.None)
                    {
                        return mainAggregation;                            
                    }

                    return pPerformanceAgg
                     .DateHistogram("date", b => b
                     .Field(p => p.date)
                     .CalendarInterval(dateInterval)
                     .Aggregations(rAgg => mainAggregation));
                })
          )));
So basically we have as result a BucketBase (in case dateAggregation is DateAggregation.None) or DateHistogramBucket/SingleBucketAggregate (in case dateAggregation is Day/Week/Year)

What I have found is there is no relation between those two bucket class types. I have just to read values and terms using the same mapping method but it is impossible to cast correctly buckets.
In conclusion, now we have two identical methods just to overcome the casting problem:

private void MapPStatistics(BucketBase aggregate)
{
var vAggregate = aggregate.Sum("v");
.......

and same but different method firm in case of DateHistogram

private void MapPStatistics(SingleBucketAggregate aggregate)
{
var vAggregate = aggregate.Sum("v");
.......
```

My question here is ..why BucketAggregateBase is not also a BucketBase? Any correct approach to avoid code duplication without instantiate new object based on a common bucket class type?

Thanks.

Breaking Discuss v8.0.0-alpha1

Most helpful comment

Thanks for raising @meriturva.

You are correct, there is currently no relation between BucketAggregateBase and BucketBase. Looking at the code it seems like we could consider consolidating this type hierarchy, which would then allow you to simply your method overloading solution.

We are currently planning breaking changes work for 8.0, so I will add this item to the discussion.

All 3 comments

Thanks for raising @meriturva.

You are correct, there is currently no relation between BucketAggregateBase and BucketBase. Looking at the code it seems like we could consider consolidating this type hierarchy, which would then allow you to simply your method overloading solution.

We are currently planning breaking changes work for 8.0, so I will add this item to the discussion.

We will address hierarchy of types in code-gen work for v8. Please let us know if this issue is still a relevant priority for your usage?

Thanks @stevedodson ...issue still relevant due to the fact that we just would like to remove few duplicated methods.

Was this page helpful?
0 / 5 - 0 ratings