Make AbstractDownsampleFieldProducer generic to faciltate more types#139033
Conversation
… to support new types
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
| final LeafDownsampleCollector.FieldCollector<?>[] fieldCollectors = new LeafDownsampleCollector.FieldCollector< | ||
| ?>[fieldValueFetchers.size()]; | ||
| for (int i = 0; i < fieldValueFetchers.size(); i++) { | ||
| var fieldValueFetcher = fieldValueFetchers.get(i); |
There was a problem hiding this comment.
Maybe for a follow-up, but would this be easier to reason about if FieldValueFetcher had a fieldCollector() method so each implementation could build a FieldCollector with the appropriate typed iterator sources?
There was a problem hiding this comment.
Let me give it some thought before I merge :).
There was a problem hiding this comment.
I took a look and it can work really well. The main idea is to make the FieldValueFetcher also generic use that in most places so that we do not need to dance between the two.
I would like to wait for the re-introduction of type specific last value producers because then it's going to work even better. Considering this a step to the right direction I will merge this. I will keep you posted.
elastic#139033) In elastic#138561 we added exponential histograms and currently we are working on adding legacy histograms too. Every time we add a new doc value reader we need to add new types of field producer and doc value reader pairs. This PR makes that easier by making `AbstractDownsampleFieldProducer` generic. This is extracted from the work to downsample legacy histograms to facilitate reviewing.
In #138561 we added exponential histograms and currently we are working on adding legacy histograms too. Every time we add a new doc value reader we need to add new types of field producer and doc value reader pairs. This PR makes that easier by making
AbstractDownsampleFieldProducergeneric.This is extracted from the work to downsample legacy histograms to facilitate reviewing.