Remove tech debt on Aggregations#getLeafCollector

[iverase]

Remove #getLeafCollector(LeafReaderContext, LeafBucketCollector) in favour of #getLeafCollector(LeafReaderContext, LeafBucketCollector, AggregationExecutionContext).

The PR is a non brainer just removing the old method and adding the new parameter to all implementations. See AggregationBase for the removal of the method.

[elasticmachine]

Pinging @elastic/es-analytics-geo (Team:Analytics)

[jpountz]

Curiosity question: should we also try to remove the LeafReaderContext from the parameter list since AggregationExecutionContext also exposes the LeafReaderContext? Or alternatively, do we really need the LeafReaderContext in AggregationExecutionContext?

[iverase]

Both are doable but not sure which one is the right answer. I think everything comes down to another tech debt issue we have in BucketCollector:

elasticsearch/server/src/main/java/org/elasticsearch/search/aggregations/BucketCollector.java

Line 44 in b55f3fb

// TODO: will remove it in a follow up PR

This seems much harder to work on as we are trying to remove a method from the interface, effectively making the BucketCollector not a collector.

[nik9000]

BucketCollector not a collector.

We can't really use it as one any more anyway. I vote we remove the super-interface, especially if we can do it in a purely mechanical way.

Personally I'd prefer to have the LeafReaderContext in the AggregationExecutionContext. And if you just need someone with an opinion use mine. But my preference isn't super strong.

[iverase]

We can't really use it as one any more anyway. I vote we remove the super-interface, especially if we can do it in a purely mechanical way.

That is my vote as well but unfortunately it cannot be removed in a mechanical way. We relay that is a collector during the query phase and changing it seems a bit of work.

[iverase]

I updated the PR so the signature of the method looks like: #getLeafCollector(AggregationExecutionContext, LeafBucketCollector).

[nik9000]

I think it's an improvement!

[iverase]

@elasticmachine update branch