Skip to content

Pass aggregator suppliers as aggregator factory constructor arguments #58136

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

Closed
not-napoleon opened this issue Jun 15, 2020 · 2 comments · Fixed by #65466
Closed

Pass aggregator suppliers as aggregator factory constructor arguments #58136

not-napoleon opened this issue Jun 15, 2020 · 2 comments · Fixed by #65466
Labels
:Analytics/Aggregations Aggregations >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@not-napoleon
Copy link
Member

Currently, during aggregator construction, we look up the supplier from the ValuesSourceRegistry twice. Once in ValuesSourceAggregationBuilder#doBuild and then again in the specific factory's doCreateInternal method. Instead, we could just pass the instance we already have from VSAB and streamline doCreateInternal. While we're touching that, we can get rid of the type checking around the suppliers. It was handy when we were writing the registry to see if we wired something up wrong, but now it's just so much boiler plate. Could be replaced with an assert or even just a bare cast.

@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jun 15, 2020
@Thlamz
Copy link
Contributor

Thlamz commented Nov 19, 2020

I want to start working on this issue! Please tell me if there is already someone else working on it, I expect to open a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
3 participants