Skip to content

Parse user type hints directly to ValuesSourceTypes #53424

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 Mar 11, 2020 · 5 comments · May be fixed by #65916 or #66662
Closed

Parse user type hints directly to ValuesSourceTypes #53424

not-napoleon opened this issue Mar 11, 2020 · 5 comments · May be fixed by #65916 or #66662
Assignees
Labels
:Analytics/Aggregations Aggregations >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >tech debt

Comments

@not-napoleon
Copy link
Member

In the current implementation, user type hints (specified via the value_type field on aggregations) get parsed leniently through the ValueType enum. The ValueType enum doesn't really add anything however, and we immediately turn that input into a ValuesSourceType. Ideally, we should just parse the user's input directly into a ValuesSourceType

While just getting parity with the current parsing would be sufficient, ideally we would tie into the ValuesSourceRegistry to allow for users hinting VSTs from plugins.

Relates to #42949

@elasticmachine
Copy link
Collaborator

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

@rjernst rjernst added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 4, 2020
@RobertoMiyoshi
Copy link

RobertoMiyoshi commented Dec 6, 2020

@not-napoleon May I try to solve this issue? #65916

@not-napoleon
Copy link
Member Author

@RobertoMiyoshi see my comments on the PR. There's a surprising amount of complexity in this issue due to our backwards compatibility policies.

@RobertoMiyoshi
Copy link

Hi Mark, thank you for your review and feedback. In this new approach, I worried about the backwards compatibility policies. The previous mapping of user type hints to the ValuesSourceType was based on a Surjective function, i.e., several of ValueTypes can be mapped to the same ValuesSourceType. In order to maintain compatibility with previous version, the PR #66662 is proposing a new mapping through a Bijection, covering the parsing and output the same json that is currently accepted, and also serializing and deserializing the same bytes.

@wchaparro
Copy link
Member

Closing as something we're not planning on doing.

@wchaparro wchaparro closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment