Skip to content
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

Format checking in InternalComposite should be done earlier #94760

Closed
romseygeek opened this issue Mar 27, 2023 · 1 comment · Fixed by #94979
Closed

Format checking in InternalComposite should be done earlier #94760

romseygeek opened this issue Mar 27, 2023 · 1 comment · Fixed by #94979
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@romseygeek
Copy link
Contributor

Description

InternalComposite.formatObject() can throw an IllegalArgumentException if the format to be applied to a composite key leads to a loss of information (eg, a date histogram with a width of one day is then given a date format of 'YYYY-mm', meaning that multiple buckets from the same month would produce the same key). This is currently only checked during serialization, meaning that we can do a great deal of work building aggregations only to then throw the result away at the end. This also causes a problem for converting the _search endpoint to using chunked xcontent serialization, as the HTTP response code is the first thing sent back, and so errors that occur during later serialization don't get handled correctly.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 27, 2023
romseygeek added a commit that referenced this issue Apr 3, 2023
Composite after_keys are currently only formatted when they are serialized
to xcontent, meaning that a key that cannot be formatted will not be caught
until after all the results from an aggregation have been reduced. This
wastes CPU time, and also blocks any move to chunked xcontent handling
in search responses.

This commit moves formatting of an after_key into InternalComposite's
constructor, catching these errors earlier.

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

Successfully merging a pull request may close this issue.

2 participants